Comments Lie!

Code never lies, comments sometimes do. (Ron Jeffries)

“Write a lot of comments”, was advice from the product manager. His intention wasn’t bad but unfortunately, comments don’t make always good code. I also used to think that comments are always good. Then I read Clean Code by Robert C. Martin, and it totally opened my view about coding (and commenting). In a nutshell, good code doesn’t need comments, but the bad code needs comments. Why? The good code tells straight what it does; it doesn’t need comments at all to tell about its function. But if the code is difficult to understand, there have to be comments to describe what code does.

In this blog post, I will write mainly about “commenting antipatterns” that we see too often in our code base. The main problem is that oftentimes comments are not up to date and they “lie”.

Automatic Header Comments Left Unhanged

This is maybe most common and even quite bad commenting antipattern. For example, in Visual Studio there is a handy shortcut to get following method header comments when we type “///”:

/// <summary>
/// 
/// </summary>
/// <param name="firstName"></param>
/// <param name="lastName"></param>
/// <returns></returns>
public string GetFullName(string firstName, string lastName)
{
    return firstName + " " + lastName;
}

This is really handy to get proper header comments format and fill the comments after that. It is not rare to see that developers leave them like that and don’t actually comment! How does the above header comments help anyone? It even makes code much worse because it takes more time to read that code because we will read both the useless and “empty” header comments and the code. We can’t be so busy that we don’t have time to fill those header comments.

GhostDoc Should be Banned

GhostDoc is a header comments tool that puts initial values to header comments. Like above automatic empty header comments, at first site, GhostDoc looks handy. Here is an example of how GhostDoc generates header comments for the same method:

/// <summary>
/// Returns the full name.
/// </summary>
/// <param name="firstName">The first name</param>
/// <param name="lastName">The last name</param>
/// <returns>Full name</returns>
public string GetFullName(string firstName, string lastName)
{
    return firstName + " " + lastName;
}

Wow! It wrote real comments on behalf of us, how handy! Truth is that is not handy at all. When we read those comments again we realize they don’t help us to understand the method. Those are just obvious generated comments and it takes much more time to read that code. We can see without comments that parameter firstName is the first name.

Problem with GhostDoc is that usually developers leave those comments untouched and leave a lot of really obvious comments in the code. It also violates very important DRY principle because comments repeat what code says (according to The Pragmatic Programmer by Andrew Hunt and David Thomas, DRY is one of the most important rules for the developers). We should consider not to use GhostDoc like tools. Instead of it, we should write good comments by ourselves.

Outdated Comments

This antipattern is most common and unfortunately, we all do it now and then: we forget (don’t bother) to update comments when we change the code. In the worst case, it is really dangerous when we read comments that practically lie.

Outdated Header Comments

Methods’ header comments get outdated now and then. We don’t bother to update them when we change method: purpose, return value, parameters etc. We should take more care on this because we read header comments daily and we make our assumptions based on them.

Here we have updated the GetFullName to include a prefix, but prefix hasn’t been commented in the header comments. Also, summary and return values are not correct anymore. (Yes, this has GhostDoc like obvious comments but they are there for simplicity.)

/// <summary>
/// Returns the full name.
/// </summary>
/// <param name="firstName">The first name</param>
/// <param name="lastName">The last name</param>
/// <returns>Full name</returns>
public string GetFullName(string firstName, string lastName, string prefix)
{
    return prefix + " " + firstName + " " + lastName;
}

Another example is removing a parameter and forget to remove it from the header comments:

/// <summary>
/// Returns the full name including the prefix (for example "Dr.").
/// </summary>
/// <param name="firstName">The first name</param>
/// <param name="lastName">The last name</param>
/// <param name="prefix">Prefix of the name</param>
/// <returns>prefix firstName lastName</returns>
public string GetFullName(string firstName, string lastName)
{
    return firstName + " " + lastName;
}

This is worse than the previous one. Header comments and code doesn’t match at all anymore. Imagine the upper method not to be that simple, and we can see there is a real risk for a big misunderstanding of the method’s purpose.

These kinds of incorrect header comments happen also when we copy paste some method and forget to update the copied header comment.

Fortunately, many IDEs underline those removed, added and renamed parameters in header comments and suggests to remove/update them.

Outdated Implementation Comments

So far these antipatterns have been something we can avoid if we are careful. We know when we have removed a parameter and can remove the parameter from the header comments, for example. But when there is an implementation comment in the method and we make some change to the method, it is not that easy to see if the comment needs to change.

public bool IsBilled(string billId)
{
    // Check from the stock repository if there is bill ID
    // that is not expired.
    return this.billingRepository.Exists(billId);
}

If we read above code and comment carefully we can see they don’t match each other. Comments say about stock repository but code uses billing repository. In some cases this can be even confusing; “is it stock or billing repository?”, “there isn’t any expiration check here?”. This is an over simplication. But if we imagine a little longer method, we can notice that this kind of outdated comments can be even really dangerous.

Suggestion: Do Not Require Header Comments

Header comments are many times handy and quite often project’s/software’s coding conventions define them as required. The purpose of required header comments is good: goal is to make code more readable via comments. It often leads to a obvious header comments that only repeat what the code already says and violates the important DRY principle. In many cases required header comments make us use GhostDoc like generated header comments which too often leaves really bad header comments.

I personally don’t like mandatory header comments for that reason. Header comments may even make code worse if we don’t put good names to methods and parameters because he can write long header comments about them.

My suggestion is that header comments are only required if the code doesn’t say what it does. If method’s and its parameters’ names are obvious, header comments shouldn’t be required. Why should we put header comments if the code obviously says what it does?

Conclusion

Writing good comments is easier said than done, and we can’t always write perfect comments. If we are careful when writing comments and code, we can improve the readability and maintainability of the code. That way comments won’t lie and they help us.

Rule of thumb: if the code needs comments, it is probably a bad code (and vice versa, if the code doesn’t need comments, it is probably a good code). If we notice we have to write a lot of comments, my advice is to refactor the code so that fewer comments are needed. Maybe first we can write the code with comments, and then refactor it so that less comments are needed (don’t forget the unit tests or we can’t refactor the code).

Ps. Interesting Idea: Write Comments First

Recently I read A Philosophy of Software Design by John Ousterhout. There was a really interesting idea: write comments first (something like “comments driven development”?). Generally, I don’t think it is a good idea because there is a big risk that code and comments finally don’t match, and comments are left not up to date. But sometimes it could be a good way to write the code. If you want to learn more about commenting, I recommend reading that book. It has more thoughtful and different ideas about commenting the code than “Clean Code” books.