• (unregistered) in reply to Stan Rogers
    Stan Rogers:

    Unsyncronized inline comments are bad, but not nearly as bad as uncommented code with missing doco. And no, API-level comments (JavaDoc and the like) are no help at all to anyone looking at the inside of a module. It doesn't help at all to know what the inputs and outputs of the black box are supposed to look like when what you are trying to fix/change lives inside the black box.


    I disagree, JavaDoc-like comments should be all a coder needs to extract the workings of a black box. Side effect and external results are part of the output and behaviour of the black box and we were talking about well written code so the method should at most contain eight statements (as a rough guideline.). Also I expect API-level documentation for all methods, even protected or private ones.<o:p></o:p>

    RC

  • (cs) in reply to

    Again, that works well if your code is trivial or obvious -- ONLY.

  • (unregistered) in reply to Stan Rogers

    <FONT size=2>"As for comments: I strongly encourage people to avoid them whenever possible."</FONT>

    When I see someone make a statement like this, all I can say is, may god have mercy on your soul. And may you one day have to maintain code written by a total novice who took your advice to heart.

    In my experience, programmers never, EVER comment their code enough. Especially the ones who don't know what they're doing.

    So, if you're a newbie programmer, and reading this thread, for sake of myself and everyone who has to maintain your code ten years from now when you've given up on coding and moved onto another company, COMMENT YOUR CODE A LOT. AS MUCH AS YOU POSSIBLY CAN. Thank you.

     

  • (cs) in reply to

    :
    In my experience, programmers never, EVER comment their code enough. Especially the ones who don't know what they're doing.

    Reminds me of once I was visiting an ex-employer.  One of the very few programmers left from when I was there (it had been 14 months!) so showing me around the place (besides having almost an entirely new staff, the department had changed floors), and introducing me to some of the new programmers.    One ask what I had worked on.  Since I'd touched most areas of the project, I offered this simple test:  "If you see a comment in the code, most likely, I wrote it --- particularly if it's sarcastic"

  • (unregistered) in reply to

    What a complete and utter dck. Unfortunately, I think the guy works in the lab two doors down from mine... [-)]

  • (unregistered) in reply to

    >This is a WTF. Why doesn't the Info() method check if IsInfoEnabled?

    Maybe because its more efficient this way? A subroutine call + a property check is more expensive than a property check alone.  If the compiler can resolve the property check at compile time then things are even better.

     

  • (cs) in reply to skicow
    skicow:
    [image] Katja wrote:

    It is weird though, but at school they hardly ever talk about adding comments to the sourcecode. They do talk about maintaining additional documentation that describes the code (technical design stuff) but not really about comments itself.

    That is weird, and a WTF too IMHO. Why wouldn't a school teach commenting? I would think that great emphasis should be placed on commenting and formatting code when teaching ANY programming language. At school we would lose lots of marks for not indenting or commenting our code.

    Well, they do teach proper indentation and tell us how important it is to write proper documentation of whatever we're doing. They just seem to ignore the fact that the best place for the very technical information is inside the sourcecode itself. We're learning how to write functional designs. We're learning how to write technical designs. But once we're told to write code, no one is encouraging us to add lots of comments to it too. As if the technical documentation we wrote should be clear enough already.

    Then again, my dad did force me to make sure I comment my own code, in those projects where I've helped him a bit. I don't like it but hey, I do comment my school projects a lot too. But I've seen works from other students and they just forget to comment it and still receive high grades...

    But as I understood it, some companies think it's a waste of time to add proper documentation since there's a deadline close by. So they encourage employees to skip it, if possible. That this also means that code becomes harder to maintain later on, especially if someone else has to take it over is often a concern for them in a few years. But right now they need to make the deadlines so they skip it...

    It's why many projects fail, I guess...

  • (cs) in reply to

    This is a common pattern, used to avoid the overhead of evaluating the log string when logging is turned off. This is important when the string contains an expression that, for instance, serialises a big data object into XML.

    Of course, the simple "string" in the above code does not need this, but you use the pattern anyway so that all the calls to logging look the same and you dont have to think about it.

  • (cs) in reply to

    To "instantiate" means "to create an instance of (an object)". You do this with the new operator (depending on the language). A variable is not the same thing as an object - it is a slot which may hold a reference to an object.

  • (cs) in reply to

    Bwahaha! Someone should tell this guy (firstly): the place where you read that? It was a humour site. That bit about "real programmers" is a joke. In fact, everything on that page (including "real programmers only use assembler") is intended as a joke.

    Secondly: coding is not a profession. It is a trade. Coding is not like writing a report or an evaluation. We build functioning artifacts. Now, you wouldn't expect a manager at O'Briens Glass to tell the glaziers how to install windows, would you? Go away. You are not helping.

  • (cs) in reply to Foon

    Foon:
    Bwahaha! Someone should tell this guy

    By "this guy", I meant "the manager".

  • (unregistered)

    Another comment related WTF I have seen is

    <FONT face="Courier New" color=#008000>//Check to see if blah blah is blah blah</FONT>

    <FONT face="Courier New" color=#0000ff>if (<put some condition here></FONT>

    <FONT face="Courier New" color=#0000ff>{</FONT>

    <FONT face="Courier New" color=#0000ff>}</FONT>

    Never minding the if usually documents itself, but the "check to see if"????

    WTF?

    "Check"? How about "test"?

    How does code "see"?

     

    I am probably nitpicking tho

     

  • (unregistered) in reply to

    That's not a complete WTF...

    For example....

    log.info(bigFatObject.toString());

    Would cause the object to generate the string even if info isn't enabled which could chew a lot of processor time depending on how big the object is.  Guarding it with the if statement will not generate the string unless logging is turned on.

    Since it's a constant being used in the example, it might be a bit WTF'ish, but I'd overlook that in a code review because it's at least consistent with guarding against unnecessary calls.

  • (cs)

    Uortunately, my school does encourage "internal commentary". However, they don't really cover proper indentation. As such, the teachers spend hours going round the class pointing out missing End Ifs and the like.

  • (unregistered) in reply to Irrelevant

    Just a few comments....

    1) Unnecessary initialization of variables in .NEt causes overhead and should be avoided!

    2) The debugger is quite capable of dealing with multiple statements on a single line (unlike many earlier versions)

    3) The watch window is completely capable of evaluating ANY expression

     

  • (cs) in reply to

    Re: "Unnecessary initialization of variables in .NEt causes overhead and should be avoided!"

    As was pointed out earlier, in C# at least, you must initialize variables prior to use to avoid the warning/error that you are attempting to use an uninitialized variable.  I've seen this happen even with simple types, and am now in the habit of initializing everything as a result.   There may be a way to turn off the warning, but it actually comes in handy sometimes (like when you meant to initialize it to a non-default, meaningful value, and forgot to.)

     

  • (unregistered) in reply to

    :
    Sometimes I wind up with unnecessary "//initialize objects" -style comments in my code because when I sit down to write a function, I'll often "sketch" the function out in pseudocode comments before writing the code

     

    There's a lot of truth in that that hasn't really been acknowledged. It's probably not this guy's reasoning but I can be sure as hell that I use this technique

  • (cs) in reply to

    Been there, done that.  When micro-managers asked me to "more fully comment code" that was already commented, I figured, as many do, that they want an additional level of explination.

    It typically resulted in many blocks such as:

    ////The below is a comment describing the code's intent

    *or*

    //The below begins with //, which causes the compiler to skip the characters that follow.

  • (unregistered)

    Why, I have an excellent way to determine which parts of my code need comments. I write the code. I make it work. Then I forget about it for a few weeks, or as long as I can afford. After that, if I can still understand what it does, fine. If not, I decipher it and add the comments.

    BTW, both the GNU coding standards and the Linux kernel coding standards have interesting things to say about comments, and understandable code in general. They're worth a look for any programmer, I would say.

  • (unregistered) in reply to
    :
    I'm sorry but I think that your comment is not polite and also not correct. Anyway the info should be written only if the Info is enabled so you have to do that checks anyway, and it's better that checks to be done by the class and not by the caller. Try not to be rude and think before you post something...

    Who are you and who are you talking to?

    Pete

  • Flights (unregistered) in reply to cjs

    good site

     

  • MrThrobbingForeheadVein (unregistered)

    Can I just say that I *hate* finding this in code 

    <font color="#008000"><font face="Courier New" size="2">    //Enter a try block
    </font></font><font color="#0000ff"><font face="Courier New" size="2">    try
        </font></font><font face="Courier New" size="2">{</font><font face="Courier New" size="2">}
    </font><font color="#008000"><font face="Courier New" size="2">    //Catch any excpeptions
    </font></font><font face="Courier New"><font size="2"><font color="#0000ff">    catch</font> (Exception ex)
    </font></font><font face="Courier New" size="2">    {</font><font face="Courier New" size="2">}
    </font><font color="#008000"><font face="Courier New" size="2"><font color="#000000">    </font>//Finally block to test the reader, if it isn't already null close it.
    </font></font><font color="#0000ff"><font face="Courier New" size="2"><font color="#000000">    </font>finally
    </font></font><font face="Courier New" size="2">    {</font><font face="Courier New" size="2">}</font>

    Its not as bad in this case,  but the problem is more obvious if the try/catch/finally were if/else if/else - what is the second comment commenting? The second block or the second and third block? Since theres no way to guess this, comments like this are often discarded by tools during refactoring, meaning I have to check the code for these and move the comment inside the block - making the scope of the comment clear - before I refactor. Thank you thank you thank you... not. Although there's worse: I've seen code like this:

    <font color="#008000"><font face="Courier New" size="2">    </font></font><font color="#008000"><font face="Courier New" size="2">            //first line of comment
    </font></font><font color="#008000"><font face="Courier New" size="2"><font color="#0000ff">    if (doofus)</font></font></font><font color="#0000ff"><font face="Courier New" size="2"> </font></font><font color="#008000"><font face="Courier New" size="2">//second line of comment</font></font>
    <font color="#0000ff"><font face="Courier New" size="2">    </font></font><font face="Courier New" size="2">{</font><font color="#008000"><font face="Courier New" size="2">            //third line of comment</font></font><font face="Courier New" size="2"></font>

    <font face="Courier New" size="2">    }
    </font>

    And the people who did this weren't even using an editor that supported rectangular selection. 

     

  • tmack (unregistered) in reply to redtetrahedron
    redtetrahedron:
    :
      wrote: This code: if (log.IsInfoEnabled) log.Info("Reading in XML");This is a WTF. Why doesn't the Info() method check if IsInfoEnabled? WTF!!!!!!!!  

    You're an idiot dude. Seriously WTF? If Log.Info check to see if it was enabled on every call you would be making tons of unnecessary checks (and potentially creating unnecessary locks if the logger is thread safe).

      He's checking log.IsInfoEnabled (which references the log object) before every call to log.Info anyway. Since it's the same object, the Info method can check the flag internally and save an object reference. It would also make the code look a little cleaner.

    Method calls are more expensive than if statment evaluations.

  • Anon (unregistered) in reply to Drak
    Drak:
    2. Can you guarantee it will be 'null' in every implementation of a C# compiler?

    Well I don't know C#, but if the C# standard says it will be initialized to null, then any compiler that doesn't is not a C# compiler.

    And if you are afraid of a compiler bug so that it doesn't follow the standard, why are you not afraid of a compiler bug that breaks assigning null (which also follows from the standard)?

    I'm a bit sick of programmers who use "but the compiler might not do it" as their last line of defense. Have them write a test program to silence them.

  • Coward (unregistered) in reply to Anon

    How is it better to write code that is implicit instead of explicit?

    Regarding the code in question you should keep in mind that it was being converted into the syntax for c# from w/e language it was originally written in.

    Furthermore management wanted the code to read this way (READ: they wanted code and comments which were explicit)

    Anon:
    Drak:
    2. Can you guarantee it will be 'null' in every implementation of a C# compiler?

    Well I don't know C#, but if the C# standard says it will be initialized to null, then any compiler that doesn't is not a C# compiler.

    And if you are afraid of a compiler bug so that it doesn't follow the standard, why are you not afraid of a compiler bug that breaks assigning null (which also follows from the standard)?

    I'm a bit sick of programmers who use "but the compiler might not do it" as their last line of defense. Have them write a test program to silence them.

Leave a comment on “Code Commentary ”

Log In or post as a guest

Replying to comment #:

« Return to Article