• fjf (unregistered) in reply to dpm
    dpm:
    Beginners comment nothing. Apprentices comment the obvious. Journeymen comment the reason for doing it. Masters comment the reason for not doing it another way. -- Richard C. Haven
    New employees comment everything. Junior programmers comment the obvious and why it doesn't work. Senior programmers comment why their job sucks. Guru programmers comment nothing. -- Robert T. Hel
  • (cs)

    On comments:

    I have a simple methodology: Write the comments for YOURSELF, so YOU can understand the code. This is a reasonable task since if you are away from the code for about 6 months, you will have forgotten the minutia of how it works, why you did it, and the errors you discovered along the way. If you write the comments for your own understanding, they will be clear in your mind, and the side effect of this is that they will most likely be clear to others as well.

    Really, a simple philosophy.

  • Dan Mercer (unregistered)

    Way back in the 90's I wrote a simple toolbar app in HP's dtksh (a Korn Shell ksh93 with extensions for the common desktop environment). We had a standard tool for launching our apps but that required a full X11 environment, which our users found intrusive, as opposed to my app which launched each app as if it was hosted directly under Windows. So, of course our user base saw it and decided to switch to it. 400 users using a quick and dirty app designed for one person's personal use. The problem - there was a bug in dtksh that caused it to crash. When I put in print statements to capture data, it wouldn't crash. Finally i put in a line with

    :

    which is a no op that returns true. And of course I had to precede it with:

    Do not remove following no op or script will fail

  • (cs) in reply to WhiskeyJack
    WhiskeyJack:
    Andrew:
    // begin comment activity 399412

    Don't you just hate it when you see this retarded shit everywhere? This is the wrong tool for the job. This is what source control history is for.

    // end comment activity 399412

    Sometimes I'll see, and add myself, comments like:

    // Added by W.Jack, 11/11/11, Problem Report 12345 // If the mode is X, then ensure Y is set

    It's also described in the version control history, as well as the problem report description, and all that good stuff, but if you're in the middle of reading/examining/editing the code, a quick 2-line note like that to remind you of some special circumstance that was taken care of, is quicker than stopping to do a diff and then firing up the reporting tools to figure out why.

    Obviously we don't go overboard, and we wouldn't bother to comment silly stuff like:

    // Oops, forgot to check for null pointer here

    // Fixed spelling mistake

    // Removed this entire function as it is no longer needed

    But the major logic bugs, "don't forget to do X before Y", etc. are helpful.

    Exactly. There are many devs here who ding me in code reviews saying we don't need a note about this, and we especially don't need to note the Jira number, because that's what subversion is for. Except. That I worked here before we had subversion (VSS before). I may work here after we move off of subversion (git maybe?). Same story with issue tracking tools. And nobody wants to go back through 5 years of revisions. Yes there's blame, but if someone else touches the line for an unrelated reason then someone else's change shows on that line. All that to save on the 'overhead' of 2 commented lines. I don't get it.

  • Pedant (unregistered) in reply to chubertdev
    chubertdev:
    I think the "don't know why this doesn't work" comments are pretty bad on their own, but definitely a WTF when compounded with the fact that they were bugs, and ones that were easily remedied.

    As for a weird issue like calling something twice to get it to work, if you can't modify the referenced method, and the double references work, then it's not a WTF.

    Well, it probably is a WTF, but it may just not be yours. :-)

  • Paul Neumann (unregistered) in reply to ObiWayneKenobi
    ObiWayneKenobi:
    The thing that really annoys me is comments that indicate the person writing the comment either had no idea what the code was doing, or worse was too lazy to refactor it to make sense.

    My old job was littered with comments like:

    // not sure where this is used

    // why is this here?

    // not sure what this code does

    // there is probably a better way to do this

    and it annoyed me because it was always the same guy, and this guy would never take 5 minutes to address it; he would copy/paste code 3+ places rather than spend the time to refactor it to a method, or just comment out a large block of code and never remove it (despite the fact we used version control) because he was worried that COMMENTED OUT CODE might break the system (no joke he said as much once).

    Bad comments are worse than no comments at all, and best of all is code that is actually readable without needing a comment to explain it. Sure, maybe have a comment if the code is for a particular workflow process, but in most cases I firmly believe code should document itself by being clear and concise.

    So, how is Amy these days?

  • (cs) in reply to airdrik
    airdrik:
    WhiskeyJack:
    Andrew:
    // begin comment activity 399412

    Don't you just hate it when you see this retarded shit everywhere? This is the wrong tool for the job. This is what source control history is for.

    // end comment activity 399412

    Sometimes I'll see, and add myself, comments like:

    // Added by W.Jack, 11/11/11, Problem Report 12345 // If the mode is X, then ensure Y is set

    It's also described in the version control history, as well as the problem report description, and all that good stuff, but if you're in the middle of reading/examining/editing the code, a quick 2-line note like that to remind you of some special circumstance that was taken care of, is quicker than stopping to do a diff and then firing up the reporting tools to figure out why.

    Obviously we don't go overboard, and we wouldn't bother to comment silly stuff like:

    // Oops, forgot to check for null pointer here

    // Fixed spelling mistake

    // Removed this entire function as it is no longer needed

    But the major logic bugs, "don't forget to do X before Y", etc. are helpful.

    I give you a 50% - The note about the reason for the fix is good. The note about who added it, when, and for which problem report is irrelevant to the code itself.
    Instead just put the reason for the fix and leave the rest of the information in the commit message. If you come to such a line and want to know who added it when for what bug ticket (or other circumstantial information about the change itself) the source history (svn and I'm sure other more sophistocated version control tools have a nice feature called Blame or Annotate which will show you who last changed each line when and what the commit message was) is not so hard to access, especially when using a modern IDE with proper VCS integration.

    Doesn't work. I change line 99 to address a security problem today. Someone else changes line 99 next month to add a new UI theme. Who shows up in blame? The hapless UI developer, who now gets a question about the access control there. I've been working on the same code base for 7 years. Change that 'next month' time period to 5 years. Maybe there is a good tool for finding out in what revision CheckAccessSecure was changed to FakeCheckAccessWideOpen, and if someone can point me to that I'd love it, but I don't know of one, especially since that could have moved from line 90 to line 99 in the same commit where the function name changed. But adding a comment line right above the function #Throwing this in for the XXX demo# will give a line whose ownership is significantly less likely to change, unless of course someone comes along and fixes the function and updates the comment

  • (cs) in reply to neminem
    neminem:
    I see absolutely nothing wrong with the existence of comments like "this is a hack", "I don't like the way this is done, but I couldn't think of a better one", or even "this isn't working right now, beware!" Yes, in a perfect world, everything would always be perfect, but that isn't the world we live in. Sometimes we need to commit things that only mostly work, or that totally work but aren't pretty, and immediately above such a code section is the perfect place to document that fact (at least assuming you aren't working with a noncompiled language where the source will be pushed directly to curious consumers :p).

    What I really love are people who feel the compulsion to comment every public method... but don't feel the same need to have those comments actually be useful, or even mean anything. I worked with one guy like that, who thought it would be a good idea to run a comment auto-generator. I guess he thought it would encourage us to replace the autogenerated comments with real ones when we saw them? Actually it just encouraged us to be amused, and then ignore them. Properties with names like FirstName, for instance, don't really need comments. And what they definitely don't need is comments like:

            /// 
            /// Gets or sets the first name.
            /// 
            /// <value>The first name.</value>
            public string FirstName
    

    But at least that's still readable. What they really don't need are comments like (real examples!):

            /// 
            /// Datas the grid view cell value changed.
            /// 
            protected abstract void DataGridViewCellValueChangedCore(DataGridViewCellEventArgs e);
    

    Or:

    /// Internals the extract icon.
    private static Icon InternalExtractIcon(string file)

    Or:

            /// 
            /// Removings the profile event handler.
    	/// 
            public delegate void RemovingProfileEventHandler(object sender, RemovingProfileEventArgs e);

    Or:

    	/// 
    	/// Toes the type of the workflow.
    	/// 
    	public static string ToWorkflowType(int pdfFieldType)
    

    Or even:

            /// 
            /// Determines whether the specified item is item.
            /// 
            /// <param name="sessionItem">The session item.</param>
            /// <returns>
            ///     <c>True</c> if the specified item is item; otherwise, <c>false</c>.
            /// </returns>
            public bool IsItem(SessionItem sessionItem)

    That appears to be C#. Every public method should have a comment for one specific reason - it shows up in Intellisense for the other developers in your company as they are using it. Given that, the other developers should come club your co-worker over the head due to the useless pop ups he's giving them.

  • (cs) in reply to Pedant
    Pedant:
    Well, it probably is a WTF, but it may just not be yours. :-)

    Haha, I stand corrected. But it's always the other guy's fault, right?

  • Paul Neumann (unregistered) in reply to wbrianwhite
    wbrianwhite:
    That appears to be C#. Every public method should have a comment for one specific reason - it shows up in Intellisense for the other developers in your company as they are using it. Given that, the other developers should come club your co-worker over the head due to the useless pop ups he's giving them.
    F*** Intellisense. If one cannot figure out that property "Name" of an instance of type "Person" is not "The name of this person." a little bubble stating as much is not going to help one to be a developer.

    Even worse is "The <see cref="Name"/> of this <see cref="Person"/>."

    Intillisense pop-ups are useless in 99% of cases I encounter. Thank god it can be turned off.

  • explain it (unregistered)

    Lines such as above

    "# Do not remove following no op or script will fail"

    are ok, except for the explanation, which might in this case be

    Because of a reproducable bug in dtksh version x.y

    Similarily for the likes of "Don't know why this call has to be used twice" which should be explained such as

    But only one call is known to not set the given values. or Because I was told to do it that way by my boss.

  • fjf (unregistered) in reply to neminem
    neminem:
    	/// 
    	/// Toes the type of the workflow.
    	/// 
    	public static string ToWorkflowType(int pdfFieldType)
    
    Works Toes of Flowtype?
  • minimaul (unregistered) in reply to neminem

    Get your developers to stop using GhostDoc then ;)

    These look a lot like GhostDoc's defaults!

  • (cs)

    Best comment I was ever responsible for had to do with branching to two different decode routines depending on what type of terminal (and hence what format) a message had come from. Type 3000 terminals would have IDs beginning with a '2', while the earlier Type 2000 terminals (typically in third-world countries) had IDs beginning with a '3'. You check the first character of the ID and proceed accordingly.

    I could have done with flashing red text in the source code to keep newbie contract programmers from "correcting" the "obvious mistake" and sending messages from terminals starting with '2' to the Type 2000 decode routine as it should "clearly be" instead of the logical criss-cross forced upon us by whoever assigned the terminal IDs in the first place.

  • fjf (unregistered) in reply to da Doctah
    da Doctah:
    Best comment I was ever responsible for had to do with branching to two different decode routines depending on what type of terminal (and hence what format) a message had come from. Type 3000 terminals would have IDs beginning with a '2', while the earlier Type 2000 terminals (typically in third-world countries) had IDs beginning with a '3'. You check the first character of the ID and proceed accordingly.

    I could have done with flashing red text in the source code to keep newbie contract programmers from "correcting" the "obvious mistake" and sending messages from terminals starting with '2' to the Type 2000 decode routine as it should "clearly be" instead of the logical criss-cross forced upon us by whoever assigned the terminal IDs in the first place.

    decode (id ^ true);
    It's correct and it looks nice. Now that's self-documenting code!

  • Minos (unregistered)

    Maybe INFERNO is what happened when someone touched the line of code that used to be there?

  • Cunso (unregistered) in reply to neminem
    neminem:
    What I really love are people who feel the compulsion to comment every public method... but don't feel the same need to have those comments actually be useful, or even mean anything. I worked with one guy like that, who thought it would be a good idea to run a comment auto-generator. I guess he thought it would encourage us to replace the autogenerated comments with real ones when we saw them? Actually it just encouraged us to be amused, and then ignore them. Properties with names like FirstName, for instance, don't really need comments. And what they *definitely* don't need is comments like:
            /// 
            /// Gets or sets the first name.
            /// 
            /// <value>The first name.</value>
            public string FirstName
    

    But at least that's still readable. What they really don't need are comments like (real examples!):

            /// 
            /// Datas the grid view cell value changed.
            /// 
            protected abstract void DataGridViewCellValueChangedCore(DataGridViewCellEventArgs e);
    

    You can blame Microsoft for that. Their standards state that every method and property in C# has to have a summary, and if you happen to use StyleCop then every method and property that doesn't have a summary will get a warning.

    This looks like someone is using GhostDoc to follow the standards, which is fine most of the time, but they're too lazy to go back and fix the comments. Most likely they just installed GhostDoc, left it on the default settings, and then they just run it whenever StyleCop complains to make it shut up.

  • NotARealName (unregistered)

    A long time ago (when I was green and stupid - now I'm yellow and stupid) I was tasked with writing a test harness for our product API. I was new to the product and didn't truly understand it but persevered nonetheless. After much head scratching and late nights I wrote a comment along the lines of:

    "Exception thrown - did you use the wrong jobtype" or something similar, as a sanity test for myself.

    7 years later I visit our customers on integration engagements and they proudly show me their web service client applications code which have this comment verbatim in there (as we distribute the test harness with the product)

    I 'might' go back and remove the comment but it must make sense to lots of people otherwise they wouldn't be including it in their own codebases, right?

  • (cs)

    But where are the unicorns?

  • Anon (unregistered) in reply to chubertdev
    chubertdev:
    I inherited a web app with a ton of these comments:
    Try
       ...
    Catch ex As Exception
       ' this should never happen
    End Try
    

    I didn't replace the comment, but when I added exception reporting after it, let's just say that we were able to solve most of the bugs in the app.

    What a shame. You should have deleted all of the try/catch and implemented unhandled exception logging in a centralized location.

    Murphy's law suggests that there will be a critical bug in the one method where you didn't have a try/catch with logging.

  • (cs) in reply to Anon
    Anon:
    chubertdev:
    I inherited a web app with a ton of these comments:
    Try
       ...
    Catch ex As Exception
       ' this should never happen
    End Try
    

    I didn't replace the comment, but when I added exception reporting after it, let's just say that we were able to solve most of the bugs in the app.

    What a shame. You should have deleted all of the try/catch and implemented unhandled exception logging in a centralized location.

    Murphy's law suggests that there will be a critical bug in the one method where you didn't have a try/catch with logging.

    Yeah, I did put exception reporting in the Application_Error event of the Global.asax file.

    I also did a Find in Files for "End Try", came in pretty handy. The app was't large enough for there to be more than a couple hundred. A little time-consuming, but well worth the effort.

  • Winner (unregistered)

    I found something like this once:

    int i = 10;  // define variable i of type int, and initialize it to 10

    Gee, thanks. There is no way in hell I would have figured that out in a million years without this immensely useful comment.

  • A commentator. (unregistered)

    Comments are what makes this job interesting.

  • fjf (unregistered) in reply to Winner
    Winner:
    I found something like this once:
    int i = 10;  // define variable i of type int, and initialize it to 10

    Gee, thanks. There is no way in hell I would have figured that out in a million years without this immensely useful comment.

    Who told you it was a comment?

    int i = 10;  /* define variable i of type int, and initialize it to 10 */  // comment the declaration
  • Charles (unregistered)

    Long, long ago, I was reading the source code listing for the Data General Eclipse AOS kernel, a 16 bit minicomputer.

    The kernel had a 16 bit status word indicating the state of various important things in the bits. The hardware had general bit testing and setting instructions but also had two special fast instructions for testing the sign bit and the bit next to the sign bit.

    The kernel defined constants for the various bits, something like:

       TTIS  =    15
       TTOS  =    14
       ALOCK =    13
    

    etc.

    Thus, you could use the constant in the general bit manipulation instructions, but not for the fast instructions, which would reference bits 14 or 15 by context. Thus, if you changed the location of TTIS or TTOS in the word, the code that was optimized for the quick instructions would break. Immediately above the constant definitions was the comment:

        ! Touch not these bits lest the angel of god strike you dead.
    

    -- Charles

  • (cs) in reply to The MAZZTer
    The MAZZTer:
    Last one probably isn't a WTF.

    Likely their server-side process replaces the text "<!-- INFERNO -->" with some real HTML before sending it client side. Crude, but it works. Some idiot probably thought it was just a useless comment and deleted it without asking anyone, hence the comment before it.

    Similar techniques have been coded, to replace a comment with a piece of functional code enabled under certain circs (did it myself in a web.xml file designed to be compatible with both a tomcat4 and tomcat5 environment, IIRC.

    The trick is to include somewhere in the vicinity a comment which actually explains what you are doing. So, not necessarily a WTF, but without that explanatory comment, fucking stupid all the same.

  • (cs) in reply to NotARealName
    NotARealName:
    A long time ago (when I was green and stupid - now I'm yellow and stupid) I was tasked with writing a test harness for our product API. I was new to the product and didn't truly understand it but persevered nonetheless. After much head scratching and late nights I wrote a comment along the lines of:

    "Exception thrown - did you use the wrong jobtype" or something similar, as a sanity test for myself.

    7 years later I visit our customers on integration engagements and they proudly show me their web service client applications code which have this comment verbatim in there (as we distribute the test harness with the product)

    I 'might' go back and remove the comment but it must make sense to lots of people otherwise they wouldn't be including it in their own codebases, right?

    No, just means it never gets triggered and nobody ever notices it.

  • HaX0r (unregistered) in reply to Surprised
    Surprised:
    Aris:
    Comments certainly don't tell what the code does. The code is there for this. Comments can tell why the code does something.

    At least, that's how I see comments ...

    I couldn't agree more.

    Except there are some 'Agile' people who out there who 'don't gold plate' and will tell you documentation is just another thing to maintain. Saying 'why' usually changes less often than 'how' but apparently we are all to stupid to understand their genius (or their Worse Than Failure code).

    Comments have many purposes (some more mundane than others). For example, I like to comment closing parenthesis. Even with an IDE that colours code and matches parenthesis, it can be difficult to work out flow in big long blocks - because little blocks become big blocks, I find commenting every parenthesis is a good habit.

    Frustrating though it is, I much prefer over commented code than undercommented code eg:

    /* Declare integer i */ int i;

    /* Set i to 1 */ i=1;

    Yes it's stupid and annoying, but I find this far easier to wade through than something with NO comments....

  • (cs) in reply to da Doctah
    da Doctah:
    Best comment I was ever responsible for had to do with branching to two different decode routines depending on what type of terminal (and hence what format) a message had come from.
    I take it we're just supposed to use the power of our imagination here.
  • Jim (unregistered) in reply to ObiWayneKenobi
    ObiWayneKenobi:
    The thing that really annoys me is comments that indicate the person writing the comment either had no idea what the code was doing, or worse was too lazy to refactor it to make sense.

    My old job was littered with comments like:

    // not sure where this is used

    // why is this here?

    // not sure what this code does

    // there is probably a better way to do this

    and it annoyed me because it was always the same guy, and this guy would never take 5 minutes to address it; he would copy/paste code 3+ places rather than spend the time to refactor it to a method, or just comment out a large block of code and never remove it (despite the fact we used version control) because he was worried that COMMENTED OUT CODE might break the system (no joke he said as much once).

    Bad comments are worse than no comments at all, and best of all is code that is actually readable without needing a comment to explain it. Sure, maybe have a comment if the code is for a particular workflow process, but in most cases I firmly believe code should document itself by being clear and concise.

    No comment is far, far worse than too much comment. Even when yoiu get the most annoying, (seemingly) useless comment, you get an insight into the type of person who wrote the code.

    Anyone who believes code can always be refactored to be readable without comments is kidding themselves. Comments aren't necessarily needed all the time, but there is always a situation where logic is complex enough to require some explanation - why this complexity exists, what it is doing, and perhaps why this approach was taken.....

    Anyone who believes NO COMMENT is a good thing (or worse yet that it is possible to write easily supportable code without ever writing a comment has obviously never had to look after code from a third party.

  • Bilongda (unregistered) in reply to WhiskeyJack
    WhiskeyJack:
    Andrew:
    // begin comment activity 399412

    Don't you just hate it when you see this retarded shit everywhere? This is the wrong tool for the job. This is what source control history is for.

    // end comment activity 399412

    Sometimes I'll see, and add myself, comments like:

    // Added by W.Jack, 11/11/11, Problem Report 12345 // If the mode is X, then ensure Y is set

    It's also described in the version control history, as well as the problem report description, and all that good stuff, but if you're in the middle of reading/examining/editing the code, a quick 2-line note like that to remind you of some special circumstance that was taken care of, is quicker than stopping to do a diff and then firing up the reporting tools to figure out why.

    Obviously we don't go overboard, and we wouldn't bother to comment silly stuff like:

    // Oops, forgot to check for null pointer here

    // Fixed spelling mistake

    // Removed this entire function as it is no longer needed

    But the major logic bugs, "don't forget to do X before Y", etc. are helpful.

    I agree. Sometimes a change is marked so that someone going through the code can go "Why is this here? Oh I see, it fixed that issue". Otherwise, how do we know whiuch version to compare to to find that fix? Was it in the last release? The one before perhaps? etc....

    Not sure you need to delimit the end, necessarily, but I have no issue with: //C453674 - Calculate GST before processing GST free items.

  • xfhxd (unregistered) in reply to anon
    anon:
    no laughing matter:
    Wronged:
    Comments should show WHY code is needed - its context in the real world, not IT speak. It should be written in a way that the business would understand.
    Probability that "the business" reads the code: <0.0001

    Probability that another developer reads (and tries to understand) the code: 1

    Probability business will ask you to describe the process they have paid for: 0.7 Probability human being will read code: 1.0 Probability they will be a good software engineer: 0.3 Probability they will be experienced: 0.5 Probability business get good value in response to their question: ? Impossible to tell, depends which Dastardly & Muttley T-shirt wearer is asked.

    If the comment already says what is happening, probability you can paste this into an email: 0.8

    And the other 0.2:

    //Fuckwit Jones from Sales insists we calculate stats this way
    // It's wrong, and the stat they get are misleading and meaningless
    // but who are we to argue with the wankers
    
  • ferdinand (unregistered) in reply to Anonymous
    Anonymous:
    ObiWayneKenobi:
    The thing that really annoys me is comments that indicate the person writing the comment either had no idea what the code was doing, or worse was too lazy to refactor it to make sense.

    My old job was littered with comments like:

    // not sure where this is used

    // why is this here?

    // not sure what this code does

    // there is probably a better way to do this

    Those comments seem normal to me. There's nothing wrong with making note of WTF code that doesn't make sense. If you don't understand it then you can't safely refactor it. It's about making note that something is fishy and you or somebody else should review it when you get a chance. It could also just be a reminder to ask somebody about it when you get a chance. I like to use #warnings for that though.
    ObiWayneKenobi:
    and it annoyed me because it was always the same guy, and this guy would never take 5 minutes to address it; he would copy/paste code 3+ places rather than spend the time to refactor it to a method, or just comment out a large block of code and never remove it (despite the fact we used version control) because he was worried that COMMENTED OUT CODE might break the system (no joke he said as much once).

    I've worked with this type of person before. Yes, they will drive you insane. The comments aren't the program. The copy/pasta, superstition, and incompetence are the problem. :P Anybody can be a code monkey, but it takes something special to be a competent programmer.
    It depends who puts the comment in.

    If the ORIGINAL AUTHOR is saying "why is this here" you have a bit of a problem (even if they added that on a revisit) If a SUBSEQUENT SUPPORT PERSON is adding that comment, then it serves as a reminder that, once we have time we probably should go nut that out......

    I recently found one (in C code) that said something akin to:

    /* The Code below is WRONG.
     * The Original Programmer wrongly assumes a 1-index on the array
     * and the seventh element sould be [6].  The current code always
     * sees a space inserted into the field, but we don't want to fix
     * it in case we break something else
     */
    

    I was a bit worried that they avoid fixing something lest they break something - surely that is a risk in anything we do. I suspect that the Business refused to let them fix it (they're always afraid the whole world will collapse ig we correct something), and I think that should have been made more clear in the comment....

    never mind.

  • hie (unregistered) in reply to Valued Service
    Valued Service:
    Wronged:
    Auction_God:
    Comments show where the bugs in the code are: The developer thought that it was difficult enough to require an explanation and therefore this is where they most likely made a mistake!

    WRONG. Comments should show WHY code is needed - its context in the real world, not IT speak. It should be written in a way that the business would understand. They should be language and implementation neutral, and concise enough to be written carefully. If your code has comments saying 'add one because we've got an offset I can't figure out' then you've got a crap team. If all your team don't comment then please let us know your company so I never apply for a job there.

    I disagree.

    The language for why should be the language that the next programmer can understand.

    Product documentation is where you write the whys that the business guys can understand. Business guys have no "business" reading your source code, unless they are fully apt developers themselves. Even then, if the VP is reading code, he doesn't trust his teams' leaders and there's a management issue.

    Not only that, but comments CAN describe what a section of code is designed to do, if the code is complex and requires that information to maintain it. Example, code that performs a complex algorithm, I usually comment what the algorithm is supposed to do and how it should accomplish that. In these cases, any sufficient self-documenting function names or class names would end up a WTF article for their names.

    In short, use common sense when writing comments, and target your audience. That's what you're taught when writing product documentation, so the same rules apply for comments, for the same reasons.

    Or put another way, code/comment it so YOU can understand it. What you do today may not be what you do tomorrow (partly because we do learn as we go along, and partly because we forget things). If you think something is unclear, explain it. If you EVER feel like "Man that was elegant/cool" - comment it - chances are when you think you're doing something elegantly, you're doing it less than intuitively. Comments are a chance for you to entertain and teach those who come after you...Excessive/useless commenting will annoy the crap out of them (as it would you)

  • sd (unregistered) in reply to airdrik
    airdrik:
    Gomer Pyle:
    The MAZZTer:
    Last one probably isn't a WTF.

    Likely their server-side process replaces the text "<!-- INFERNO -->" with some real HTML before sending it client side. Crude, but it works. Some idiot probably thought it was just a useless comment and deleted it without asking anyone, hence the comment before it.

    except that is doesn't say why INFERNO is needed... and when it would no longer be untouchable.

    Well, the time when the INFERNO thing is no longer untouchable is the time when they rewrite the code, which while that is not going to happen any time soon still has a much greater chance than that anyone is going to update the comment with a better explanation.

    It's innocent. It's a comment that costs nothing to leave there (ok a dozen or so bytes, but who's counting?) - even if it no longer does anything.

  • Mickey D (unregistered) in reply to airdrik
    airdrik:
    WhiskeyJack:
    Andrew:
    // begin comment activity 399412

    Don't you just hate it when you see this retarded shit everywhere? This is the wrong tool for the job. This is what source control history is for.

    // end comment activity 399412

    Sometimes I'll see, and add myself, comments like:

    // Added by W.Jack, 11/11/11, Problem Report 12345 // If the mode is X, then ensure Y is set

    It's also described in the version control history, as well as the problem report description, and all that good stuff, but if you're in the middle of reading/examining/editing the code, a quick 2-line note like that to remind you of some special circumstance that was taken care of, is quicker than stopping to do a diff and then firing up the reporting tools to figure out why.

    Obviously we don't go overboard, and we wouldn't bother to comment silly stuff like:

    // Oops, forgot to check for null pointer here

    // Fixed spelling mistake

    // Removed this entire function as it is no longer needed

    But the major logic bugs, "don't forget to do X before Y", etc. are helpful.

    I give you a 50% - The note about the reason for the fix is good. The note about who added it, when, and for which problem report is irrelevant to the code itself.
    Instead just put the reason for the fix and leave the rest of the information in the commit message. If you come to such a line and want to know who added it when for what bug ticket (or other circumstantial information about the change itself) the source history (svn and I'm sure other more sophistocated version control tools have a nice feature called Blame or Annotate which will show you who last changed each line when and what the commit message was) is not so hard to access, especially when using a modern IDE with proper VCS integration.

    all the more reason to include the change number - because it gives you something to search in the code (date and name maybe less relevant so I'll agree with you on that - but some of us like to see our name in lights allt he time). If this fix is a small thing tacked to a larger change, then it might not be obvious which revision refers.

  • Johnson P Finklewheel (unregistered)

    I am aghast reading through most of these comments, because it immediately becomes clear that many people here have (or think they have) worked in development rather than support (although I suspect for many of you, "worked" = written a Tic Tac Toe game for my little sister using drag and drop technologies).

    Comments explain what is going on. Even the most fluent programmer in any language won't immediately understand everything about a piece of code no matter how well/self-documenting you think you wrote it. Of course, given time, many could probably nut out what's going on - but the longer it takes them to fix an issue, the more they will curse (to themselves, to their peers and to the business) the "numpties" that created the unmaintainable spaghetti they have inherited. In some respects, you may not care because you've moved on, but surely your pride dictates that:

    1. You brag about the bits you think are clever (which is good, because it probably means it's not obvious)
    2. You explain the bits that feel "hacky" (there may well be a better way that you didn't know)
    3. You note curious constructs/patterns that are included because they inexplicably make the world happier
    4. You comment about code that is the result of (usually unreasonable) requests from some business area - especially when you feel, well - you know when you just feel they're asking for something stupid, but you can't really put your finger on what's so wrong with it
    5. You mention any point where you think "this could be a bit wierd/complex" etc

    It's no fine art, but if you sometimes revisit stuff you've written (like way back when) you start to get a feel for whoch of your comments were good, and which are stupid - and over time, you can refine your behaviour to generally write good comments when they're appropriate and leave the crap ones out. Of course, none of us can ever claim to have perfectly coded or commented something - because what seems trivial today might be of utmost importance tomorrow.

  • gnasher729 (unregistered) in reply to Winner
    Winner:
    I found something like this once:
    int i = 10;  // define variable i of type int, and initialize it to 10

    Gee, thanks. There is no way in hell I would have figured that out in a million years without this immensely useful comment.

    I wrote something like this once:

    // Do not change this - it works around a compiler bug
    int fifteen = 15; 

    followed by some code that used the variable instead of the literal. And yes, it did work around a compiler bug.

  • Jimmy (unregistered) in reply to Winner
    Winner:
    I found something like this once:
    int i = 10;  // define variable i of type int, and initialize it to 10

    Gee, thanks. There is no way in hell I would have figured that out in a million years without this immensely useful comment.

    I will admit that I have been guilty of such things in the past.

    It started at 'varsity. I had a lecturer complain that code was insufficiently commented (TBH he had a point, but that's hardly the point here) and I asked what we expected. Naturally that was a bit of a difficult question for him to answer, so I commented EVERYTHING...the habit stuck for my first few major projects in "The Real World" (tm).

  • gnasher729 (unregistered) in reply to ferdinand
    ferdinand:
    I was a bit worried that they avoid fixing something lest they break something - surely that is a risk in anything we do. I suspect that the Business refused to let them fix it (they're always afraid the whole world will collapse ig we correct something), and I think that should have been made more clear in the comment....
    Makes sense if you find a problem shortly before a release, after all testing has been done. You decide what's worse: Living with the bug in the next release, or the (small) risk that someone blunders and breaks something important. That kind of code should be fixed _immediately_ after a release is shipped.
  • Captain Oblivious (unregistered) in reply to Sergeant Obvious
    Sergeant Obvious:
    What are the chances that a developer, that can understand the IT-speak that is the code, was given tasking in business terms? Just because I can figure out what a block of code does doesn't mean I don't need to understand the business logic that makes that code necessary.

    If you don't understand the business logic, you have no business implementing the logic. Business logic and policy are distinct. "Business logic" is the configurable portin

    This is the same reason why you use variable names that are descriptive of the business logic: a=b*c has no inherent meaning, but amountBilled=multiplierDiscount*amountTaxed does. Which code would you rather see when tasked to ensure the discount does not exceed 15%?

    What is the "same reason why..."? You aren't making any sense. You say you don't need to understand the policy that lead to a plan, and then say the use of domain specific variable names is induced by "the same reason".

  • Norman Diamond (unregistered) in reply to fjf
    fjf:
    Jazz:
    Really, Remy? That's the only comment you put in this one?

    I can't tell if you're taking a minimalist tack with your humor, or if you're just losing your touch... ;-)

    His attempt to go meta ...

    But it's not a good comment: It tells us us just that he wouldn't put those things in an article directly, not but why (which is something I've really never understood).

    That's your only comment on the comment on the comment on the comment? And you only told us what, not why?

    Well, this is my only comment on the comment on the comment on the comment on the comment. There, I've just said what but not why. /* I don't know why I did that. */

  • (cs) in reply to Cunso
    Cunso:
    You can blame Microsoft for that. Their standards state that every method and property in C# has to have a summary, and if you happen to use StyleCop then every method and property that doesn't have a summary will get a warning.

    You realise you can actually (gasp) modify the settings of stylecop and turn off warnings your team thinks are not necessary.

  • Norman Diamond (unregistered) in reply to ObiWayneKenobi
    ObiWayneKenobi:
    The thing that really annoys me is comments that indicate the person writing the comment either had no idea what the code was doing, or worse was too lazy to refactor it to make sense.

    My old job was littered with comments like:

    // not sure where this is used

    // why is this here?

    // not sure what this code does

    // there is probably a better way to do this

    How about a comment in a driver file for hard disks, "BUGBUG what does this do?".

    http://blogs.msdn.com/b/larryosterman/archive/2005/03/29/403439.aspx

    By the way, does anyone know why Akismet is here?

  • Trasvi (unregistered) in reply to Quinnum

    Except when one of your business metrics for 'good quality code' is 'StyleCop under XXX settings returns no warnings'. Its far easier to write dumb comments than convince the head of development to change metrics.

  • Trasvi (unregistered) in reply to Johnson P Finklewheel
    Johnson P Finklewheel:
    I am aghast reading through most of these comments, because it immediately becomes clear that many people here have (or think they have) worked in development rather than support (although I suspect for many of you, "worked" = written a Tic Tac Toe game for my little sister using drag and drop technologies).

    Comments explain what is going on. Even the most fluent programmer in any language won't immediately understand everything about a piece of code no matter how well/self-documenting you think you wrote it. Of course, given time, many could probably nut out what's going on - but the longer it takes them to fix an issue, the more they will curse (to themselves, to their peers and to the business) the "numpties" that created the unmaintainable spaghetti they have inherited. In some respects, you may not care because you've moved on, but surely your pride dictates that:

    1. You brag about the bits you think are clever (which is good, because it probably means it's not obvious)
    2. You explain the bits that feel "hacky" (there may well be a better way that you didn't know)
    3. You note curious constructs/patterns that are included because they inexplicably make the world happier
    4. You comment about code that is the result of (usually unreasonable) requests from some business area - especially when you feel, well - you know when you just feel they're asking for something stupid, but you can't really put your finger on what's so wrong with it
    5. You mention any point where you think "this could be a bit wierd/complex" etc

    It's no fine art, but if you sometimes revisit stuff you've written (like way back when) you start to get a feel for whoch of your comments were good, and which are stupid - and over time, you can refine your behaviour to generally write good comments when they're appropriate and leave the crap ones out. Of course, none of us can ever claim to have perfectly coded or commented something - because what seems trivial today might be of utmost importance tomorrow.

    /* Best comment yet. */

  • Barf 4Eva (unregistered)

    /**

    • Please be carefull when modifying this function. This is really advanced code.
    • @return bool */ public static function initResources(){ return true; }

    I wonder if your version control system has an older version of this that DID have "advanced" code in it, but another developer came along and stripped everything but the comment and returning true...

    What if they did it to leave as a clue for some sleuth to come along and find, digging through the history so that they may discover utter sorrow, pain, and misery along their forsaken way?

    If the OP would be so kind as to check, it could add to the beauty of the WTF.

    Hopefully I'm not getting my hopes up!

  • Barf 4Eva (unregistered) in reply to Trasvi
    Trasvi:
    Johnson P Finklewheel:
    I am aghast reading through most of these comments, because it immediately becomes clear that many people here have (or think they have) worked in development rather than support (although I suspect for many of you, "worked" = written a Tic Tac Toe game for my little sister using drag and drop technologies).

    Comments explain what is going on. Even the most fluent programmer in any language won't immediately understand everything about a piece of code no matter how well/self-documenting you think you wrote it. Of course, given time, many could probably nut out what's going on - but the longer it takes them to fix an issue, the more they will curse (to themselves, to their peers and to the business) the "numpties" that created the unmaintainable spaghetti they have inherited. In some respects, you may not care because you've moved on, but surely your pride dictates that:

    1. You brag about the bits you think are clever (which is good, because it probably means it's not obvious)
    2. You explain the bits that feel "hacky" (there may well be a better way that you didn't know)
    3. You note curious constructs/patterns that are included because they inexplicably make the world happier
    4. You comment about code that is the result of (usually unreasonable) requests from some business area - especially when you feel, well - you know when you just feel they're asking for something stupid, but you can't really put your finger on what's so wrong with it
    5. You mention any point where you think "this could be a bit wierd/complex" etc

    It's no fine art, but if you sometimes revisit stuff you've written (like way back when) you start to get a feel for whoch of your comments were good, and which are stupid - and over time, you can refine your behaviour to generally write good comments when they're appropriate and leave the crap ones out. Of course, none of us can ever claim to have perfectly coded or commented something - because what seems trivial today might be of utmost importance tomorrow.

    /* Best comment yet. */

    /* Agreed. */

  • atk (unregistered) in reply to Captain Oblivious
    Captain Oblivious:
    Sergeant Obvious:
    ...Just because I can figure out what a block of code does doesn't mean I don't need to understand the business logic that makes that code necessary.

    If you don't understand the business logic, you have no business implementing the logic. Business logic and policy are distinct. "Business logic" is the configurable portin

    ...

    Let's refactor the good Sgt's first sentence and remove the double negative...

    "...does doesn't... don't...." should become positive "does" for something like "Just becuase I can figure out what a block of code does, I should understand the business logic" But that sounds weird. Refactoring a bit more, "Even if I can figure out what a block of code does, I should understand the the business logic." Kinda matches your point.

    Sgt, please avoid double negatives. It helps communication.

    Cpt, please note double negatives and engage your secondary lexical parser. It's less performant but working slightly harder to understand others helps communication.

    Communication means we can address more interesting problems, like why TDWTF stories have been going downhill for so long.

  • Evan (unregistered) in reply to Barf 4Eva
    Barf 4Eva:
    /** * Please be carefull when modifying this function. This is really advanced code. * @return bool */ public static function initResources(){ return true; }

    I wonder if your version control system has an older version of this that DID have "advanced" code in it, but another developer came along and stripped everything but the comment and returning true...

    My immediate guess was it was just sarcasm on the part of the developer. If I had to spin a whole story, I'd guess that either (1) initResources() used to do stuff (but not necessarily complex) but was changed so that it didn't or (2) it was added preemptively as they thought that it might be necessary in the future, and the comment was added by either the dev who removed the stuff in case (1) or wrote the function in case (2).

    atk:
    Sergeant Obvious:
    ...Just because I can figure out what a block of code does doesn't mean I don't need to understand the business logic that makes that code necessary.
    Let's refactor the good Sgt's first sentence and remove the double negative...

    "...does doesn't... don't...." should become positive "does" for something like "Just becuase I can figure out what a block of code does, I should understand the business logic" But that sounds weird. Refactoring a bit more, "Even if I can figure out what a block of code does, I should understand the the business logic." Kinda matches your point.

    Since you brought up the issue, I rather disagree with this part of what you say. I mean, your version is true statement that Sergent Obvious would likely agree with, but I'd say it has a different connotation. Sgt's original has much more of a flavor of "understanding the code doesn't mean I understand the business logic" which I think is largely absent from your version. In that sense, I think Sgt's original is clearer.

    But that's not to say another version might be both clearer and come across the right way.

    (And the double-negatives has implications for coding style too -- in particular, I find it rather easier to understand Booleans that are named in the positive sense. I've worked on a code base that's essentially an emulator and contained a field called isNotHalted... and you can only see if(!isNotHalted) so many times before you want to get a bit stabby and wish it had been called isRunning. :-))

Leave a comment on “Comments”

Log In or post as a guest

Replying to comment #:

« Return to Article