[carsongross]

God forbid someone gets their thoughts together with comments outlining what a function is going to do and then fills in the code...

Or, while deep in thought, someone taps out a bit of redundant commentary on a line or two of their code...

Or someone needs a bit of natural language to provide an anchor in their code.

The important thing here, I think we can all agree, is that we are smarter than that person.

[swanson]

    // don't use hardware acceleration
    canvas.enableHardwareAcceleration(true);

Which is true? The comment is lie. Is this a bug? Was this changed for some reason? Should I remove the comment? Should I toggle the boolean? Why weren't we supposed to use HW acceleration? Why are we using it now? What changed? What value did the comment have in the first place?

[lhnz]

  // Hardware acceleration should be switched off.
  // Originally we'd planned to use hardware acceleration but later
  // on in the project's life cycle we were told we needed to support older
  // versions of Foo: X and Y. This caused some bugs documented [here](#123) and [here](#234)
  // due to incomplete support of feature Z and the way that [internMethod()](#internMethod) is currently implemented.
  // @TODO: It would be better if we could switch it on or off depending on the device.
  canvas.enableHardwareAcceleration(true);

Comments should be organic and give context.

They should describe why and the high-level how, while code should describe what and the the low-level how.

That's just my opinion. This is the kind of comment I write. From time to time it becomes incorrect when a project's moving fast, but I write enough that it's very easy to validate that the use cases and code I'm talking about are real. You're meant to delete the comment immediately if it's incorrect: you can choose to help the next person understand the code or not, that's not up to me.

I think something that's very important is trying to show where a line of code or method fits in the greater scheme of the architecture of the code by linking it towards the methods that are interrelated in important ways.

Edit: You might notice I've left the comment not matching the code. This is a fact of life: somebody will change the code without updating the comment. However, because of the comment you now have a great way of sanity checking this change without having to find the original coder or test it all. Basically: it tells you what you couldn't quickly know looking at uncommented code.

[IgorPartola]

Let's look at some stylized comments I've seen over the years:

    // Add tax
    total = price + tax

Useless comment, isn't it?

    // Iterate over the order items
    for item in order:
        ...

Also useless.

    for ($i = 0; $i < count(points); $i++) {
        if (...) {
            // Break out of the loop
            $i = 1000000;
        }
    }

Not as useless as the code below it, but still pretty useless.

    def add_item(order, item):
        '''Appends item to order.

        param order : instance of <Order>
        param item : instance of <OrderItem>
        return : None
        ''''

        order.append(item)

This is a great example of a useless docstring. The developer might as well have put the code into the docstring.

    # <HACK-ALERT>
    # The code below uses an undocumented API, that nonetheless we must use in order to make the code work.
    # As per <link to StackOverflow> and <link to unresolved ticket>, this is a known issue.
    # We may be able to remove it after release X.Y where the upstream fixes it.
    
    Widget._enableDitzelMode(Widget._DITZEL_MODE_FLAG_X)

    # </HACK-ALERT>

This is not only useful, but more or less required. If you do any kind of monkey patching, please add this.

    # <DANGER>
    # The code below looks wrong, but is in fact correct. This @#$%ing API makes you say "true"
    # when you mean "false". Do not alter without reading <link to docs>.
    # Set PRODUCTION mode to ON.

    Widget.setStagingMode(true)

    # </DANGER>

This is also pretty much required, I think.

    // Make sure to use === below since foo can be null OR '' which mean different things for us:
    if (foo === null) {
        alert("Value is unknown.");
    }
    else {
        processValue(foo);
    }

I'd say also useful, but the code is going to change eventually.

[millstone]

Sure. Let's say the author intended to disable hardware acceleration, but introduced this bug. A reader might notice the disagreement between the comment and the code, and investigate.

Now take away the comment. You would never know from reading the code that there might be a problem.

This is a bad comment, but it's still more useful than no comment.

[msluyter]

I think it may be possible for all of the following to be true:

1. Comments explaining the 'why' can be helpful.

2. However, due to their inherent DRY violation, comments may become out of sync with the code and impose maintenance costs.

3. The value of comments still outweighs their maintenance costs.

That is to say, an example of a misleading comment doesn't necessarily prove that comments are bad in general. The converse may also be true, however. I no longer have a strong stand on this issue. I try to generally go with whatever commenting style the project I'm on uses, though I also have been known to write the occasional long descriptive pre-amble.

[tetrep]

>What value did the comment have in the first place?

That comment is the only indicator that something may be wrong. Your next steps are the same as with any suspected bug, find out what the code is doing (seems obvious here) and then find out if that's what it should be doing.

That comment, however, is a good example of a bad comment. In this particular scenario, it's very obvious what the code is doing, so the comment should describe why the code is doing what it's doing. Much like should do:

    //set volume to 5
    x = 5;

Rather than:

    //set x to 5
    x = 5;

[glimcat]

> That comment, however, is a good example of a bad comment.

Yes, assuming there's not some contextual reason why hardware acceleration is obviously inappropriate. But even then, it would be much better to err on the side of "assume the next person to read it hasn't had their coffee yet."

Even something like "don't use hardware acceleration because compatibility issues" would be a better comment. You don't need an essay. But it should be both concise AND descriptive of the issue, not just normative or prescriptive.

[swanson]

Nope. You should do

  volume = 5;

Or

  setVolumeLevel(5);

[glimcat]

Almost every time I've done this, that pattern means "I really shouldn't be doing it this way, but we'll do it this way for now because getting it done is more important. Improve it if emergent bugs or profiler results ever indicate it's worth messing with again, but I'll probably get back to it never."

It's not a TODO or a BUG, but it's helpful to make note of for future code review or reuse.

[swanson]

Do you always do project management/backlog grooming inside your source code? ;-)

[glimcat]

I'm of the philosophy that code should be readable, reasonably self-documenting, and remind you of relevant external material - whether that be your state of mind at the time, or reference material you drew on, or whatever.

Not everything needs an open issue, but the knowledge should be recorded somewhere. I know for a fact I won't remember it in six months, and likely not in a week either.

(I also write a large amount of utility code that's below the "project" scale and will probably never see a PM tool, but which is nonetheless part of a heavy reuse ecosystem.)

[freehunter]

I've got some code that looks like

//don't do this

do.this(true)

To me, that signifies that I know the way I'm doing it is wrong, but it works until I can figure out how to do it right.

Yes, I would be a horrible collaborator on a project. Most hackers would be, for the definition of hackers meaning "people who do isomething because it works, not because it's good programming".

[andrewSC]

Maybe a better way to clarify it's not ideal is to make a TODO. So:

// TODO: Find out a better way to do this

do.this(true)

[glimcat]

TODO statements should be conserved to actual pending tasks.

Code in this pattern is often a compromise, and may be found as contributory to future issues, but it probably didn't merit a follow-up task at the time of writing.

[Ellipsis753]

It's a pain but check the git history.

[sanderjd]

It's not a pain. Source control history is the ultimate commentary. But the same ideas apply to commit messages as to source comments - if they say what instead of why, they don't help much.

[swanson]

Both lines were committed at the same time in a changeset with 15 other files. What now?

[ww520]

Looks up the spec to see the correct behavior, just ask the person did the change, or test it yourself to see why is there a contrasting comment. May be hardware acceleration was meant to be turned off but the guy was testing it by toggling the flag and forgot to set it back. In that case the conflicting comment brings the potential bug upfront.

[andrewSC]

Find out who the author was via git blame and ask what they were thinking when writing it. Or in the case that the person who wrote it left, contact someone with some knowledge of the line in question (i.e. the person who either also wrote the majority of the code or is at least familiar with what it does) and ask. If it comes down to it and you're the only one qualified to do something about it, then just remove the line (assuming the code is functioning properly) just how I would approach it.

[CodeMage]

You're completely right, there is no sense in pointing out what kinds of comments detract from readability and anyone writing about that is a smug jerk.

All sarcasm aside, there's nothing wrong in getting your thoughts together in comments before you actually write code. What the author objects to -- and I agree with that objection wholeheartedly -- is leaving those comments in your code once you're done coding.

Other comments describe and discuss the dangers and downsides of leaving the "what" and "how" comments, so I won't go there. Instead, I'd like to propose a habit to establish when writing code: when you're done coding, remove all the "what" and "how" comments and try to read the code. If you can leave it overnight before trying to read it, even better. If it still looks easy to understand without those comments, then there's a chance it might be good enough for other people. Otherwise, do whatever you can to make it more understandable -- break it up into smaller methods with descriptive names, for example.

The point I'm trying to make is that there are two kinds of WTFs I get when I'm trying to read code: the "why the hell did they do this" WTF and the "what the hell does this do" WTF. The former is the kind of WTF you have to solve with comments, because no amount of comments is going to explain why you've done things this way, whereas the latter should be solved by writing more readable code.

[ebiester]

Not at all. I use comments like this frequently.

It might even be checked in while development is happening.

However, the "whats" are taken out completely - the tests are better places to figure out how it works, anyway. The "hows" are mostly taken out -- if someone needs to know how it's working, it needs to be refactored. (And that's why I wrote the tests!)

What remains are the "whys." Why was this algorithm chosen? Why does this look seemingly broken? Why is this code here? (IE, performance improvement led to a little less clean code.)