3 Misuses of Code Comments

Anthony Caliendo, Wednesday, July 1, 2009 @ 7:44 am

Photo Credit: by dpascoe

Not many people talk about good practices to use for comments in code. Many people see them as an extra or freebie, so not a lot of thought gets put into them. The truth is, though, that comments are a great tool for giving insight into the thought process when the code was being written. Unfortunately like any tool, comments can be misused.

Here are three of the most common misuses of comments which I tend to run into.

1. Commenting the Obvious
A.K.A. Superfluous Comments
A good rule of thumb for comments is to not comment on what the code is doing, but why it is doing it. You then try to write the code as self-documenting as possible so that footnotes aren't required to figure out what is going on. Obviously, there are going to be some times when this rule can't be followed because of the complexity of the situation and in those cases comments about what is going on is valuable. However, you will sometimes run into comments for code which is easy to understand

Please take a look at the following real live example from some Flex 3 SDK code which sparked the idea for this post. I would have to assume that anyone familiar with ActionScript would be able to figure it out on their own...
BAD

 
    //--------------------------------------------------------------------------
    //
    //  Constructor
    //
    //--------------------------------------------------------------------------
 
    /**
     *  Constructor.
     */
    public function TextInput()
    {
        super();
 
        // InteractiveObject variables.
        tabChildren = true;
    }
 

When I saw that first giant comment, I thought to myself "hey, this just might be a constructor". When I saw the second comment, I was sold on the idea! Nestle that third comment about InteractiveObject variables into your mind for the next misuse (misleading comments), but try to focus on the first two for this one.

Commenting on very obvious stuff such as "this is a constructor" just creates clutter which reduces the readability of the code. Instead of seeing more code at once, I have to scroll past large chunks of useless information. Whatever "code style" reasons people use as rationalization, these types of comments are unnecessary and don't provide useful additional information or clarity (the whole point of comments). If you feel you need a map to show where things are located, I would suggest refactoring into smaller classes and/or establishing code formatting guidelines (such as, "static variables go at the top above the constructor").

So, knowing this, what should the comments look like?
BETTER

 
    public function TextInput()
    {
        super();
 
        // We want our children to be tab-enabled since our TextField
        // child will be handling a lot of the work, so force this
        // value to true to allow that.
        tabChildren = true;
    }
 

Now, my attention is focused on a useful comment and not parsing through a lot of useless comments.

2. Comment Confusion
A.K.A. Incorrect or Misleading Comments
There are two main causes for this problem. The first cause is if the coder will write a comment that is either incorrect or not very obvious. Sometimes this is because the confusing comment makes sense to the person writing it, and sometimes it is just because the person made a mistake. The second cause is "code/comment drift"; this is when code is updated but the comments are not. Code can change over time, but sometimes the comments that explain what is going on are left behind when the code changes to do something else.

Whatever the cause, incorrect or misleading comments can lead to confusion when the comment contradicts the code in subtle (or not so subtle) ways; you have to dig through source control or old requirements (which are sometimes not updated either) to figure out what the code should be doing. It is an even worse problem if there is no test for the logic in question.

Deleting or updating comments when you change the code seems obvious to most, but I have run into this problem on more than one occasion. The same goes for writing clear comments. I could have used the previous example's comment of "// InteractiveObject variables." to illustrate this point since that is confusing given what the code which follows it actually does , but I'm going to spoil you with an exaggerated fake example.
BAD

 
public int count(List<Something> stuff) {
    // the number of foos we find
    int barTotal = 0;
 
    // count foos
    for (Something it : stuff) {
        if (it.isBar()) {
            barTotal += 1;
        }
    }
 
    // return foo count
    return barTotal;
}
 

Now, judging by the comments, the code has a bug since it should be counting foos and not bars. In a perfect world, the method would be named better to indicate what needs to be counted (self-documenting code) and there would be a unit test which would prove what should be going on. But since we don't have any of that, we are left scratching our heads and spending time looking in source control and old requirements documents to try to figure out what needs to be counted.

What would some better comments look like? (N.B. I won't change any of the code's naming, which is really what needs to be done in this case).
BETTER

 
/**
  * Count the number of bars in the passed list, and return it
  */
public int count(List<Something> stuff) {
    int barTotal = 0;
 
    for (Something it : stuff) {
        if (it.isBar()) {
            barTotal += 1;
        }
    }
 
    return barTotal;
}
 

Notice that I made the comment reflect the code. In the absence of any evidence of what the code should be doing, assume that it is doing the right thing and adjust the comments to reflect that. This piece of code still needs to be investigated to determine if it is a bug or not (no, seriously, just changing comments is sweeping the problem under the rug), but at least the comment confusion has been removed.

3. Not Enough Comments
A.K.A. Failure to Execute
This is probably the most common situation. When writing code, people don't tend to think about maintaining the code or they have difficultly realizing what "unique knowledge" they have about the situation which makes certain things obvious to them but completely confusing to someone else.

Knowing when a comment is needed is an art and it is difficult to give direction. The one piece of advice which I've found useful is to ask yourself the question "if I came back and looked at this code in a year, would I know what is going on?". If not, try to leave comments which would help your future self figure that out.

Take a look at the following:
BAD

 
public void handleError(int errorCode) {
    if (errorCode < 3) {
        errorCode += 1;
    }
 
    notify("received error" + errorCode);
}
 

Now, why exactly do we need to add one if the error code is less than three? What sort of madness is behind this?

Without a comment to tell someone what is going on (or remind yourself), this magic math is a mystery. Do we need to do this? Has the reason for this apparent hack changed since it was coded up? Keep in mind that a unit test wouldn't be much help here unless it had comments or a name which implied why this is being done.

Now, let's take a look at the same code which different comments.
BETTER

 
public void handleError(int errorCode) {
    // to support operations, our error codes need to be translated
    // to their scheme.  the mapping is:
    //     our 3 and up = the same for operations
    //     our 0 = their 1
    //     our 1 = their 2
    //     our 2 = their 3
    if (errorCode < 3) {
        errorCode += 1;
    }
 
    notify("received error" + errorCode);
}
 

With just a simple comment, the reasoning for this math is now known to anyone who looks at the code. A year later, you will be able to look at the code and know why that math is being done.
UPDATE: 3:30PM CT: Again, the correct solution to this problem would be to refactor the code to be self-documenting; however, I wanted to focus mainly on the comment aspect of this instead of improving the code itself. Improving the code would make the comment completely unnecessary, but ignore that for the sake of this example .

Good coding practices, TDD, and pair programming will play a major role in alleviating comment misuse. However, keeping in mind the rule of commenting the why and not the what will go a long way to improving comment quality.

Related posts:

1. Free Book Redux
2. See the Code, Be the Code
3. Anti-patterns in Code: Ubiquitous HashMap
4. Really Simple History 0.6 RC1 now live at Google Code
5. Grails: Delegating to GORM Persistence in Java

Comments (5)

Software Development

Comments: 5 so far

• Leave a comment
• Comment feed for this post
• TrackBack URL

1. 

   I can’t remember where I read it, but the best advice I’ve seen for when and what to comment is to only write comments that include the word “because”. Comments are really only useful for explaining strange application behavior.

   Comment by Tammer Saleh, Wednesday, July 1, 2009 @ 1:18 pm

2. 

   I agree with most of this article. My philosophy is that code that’s hard to understand should be commented. But code that’s hard to understand should be avoided.

   My main complaint about comments is that they’re not compilable. The comment can be completely wrong, but no test will catch it. So I prefer to express intention in identifier names, where at least there is a compilable correlation between uses.

   For example, in the last example, the comment can be avoided simply by putting the affected code in a method called something like map_their_error_codes_to_ours(). By factoring out functionality, the code is easier to understand, even without any comments.

   Comment by Mark Wilden, Wednesday, July 1, 2009 @ 2:12 pm

3. 

   @Mark Wilden: I agree 100%. When possible, self-documenting code should be used over comments. If the code itself is easier to understand, then you don’t need to worry about the comments.

   I think my last example was a little weak and didn’t show my point exactly, but I was trying to focus more on what to do with only the comments and not improve the code quality at all. I think I’ll update my post to highlight that.

   Comment by Anthony Caliendo, Wednesday, July 1, 2009 @ 2:20 pm

4. 

   I’ve found that comments are a smell, indicating that the code might need refactoring.

   Comment by Mark Wilden, Wednesday, July 1, 2009 @ 3:58 pm

5. 

   I’ll throw in my two cents and agree with Mark. Nothing drives me nuttier than reading through code that has twice as many lines of comments as code. Well chosen variable and method names can get you a long way in making the logic more understandable. I prefer to comment before the method/function and let the code speak for itself. Good posting, Anthony.

   Comment by Matt Hargus, Tuesday, July 14, 2009 @ 3:03 pm

Leave a comment