Treatise on Comments

|

When we started a new "continuous improvement" biweekly session within my team, my manager asked me to cover "comments" as the first topic. At first I wasn’t too excited about it – couldn’t we do something more technical? – but I realized that this was actually a great topic to start with and something from which the whole team could benefit.

In fact, comments end up being the #1 thing I comment on when I review merge requests. Now a part of that is certainly due to the way we’ve integrated Spotbugs, Checkstyle, Error Prone, and other static analysis checks into our automated Jenkins builds – many errors in the code itself never make it to me as the reviewer. But when you dive into it you realize that comments can make a huge difference in the quality of your code and APIs.

I’m going to cover both inline comments and Javadoc comments in this post. Disclaimer: these guidelines are based on Clean Code and Effective Java, so don’t just take my word for it. I’ve also been guilty of all the bad examples that I present!

But I’m a programmer and I hate writing … why do comments matter?

Before we get into this, you’re probably wondering why anyone would be so picky about comments. The answer boils down to three things.

First, I would say that being a better writer will make you a better developer. Good programming and good writing are both about clarity and economy of expression. Ultimately, you can’t write good code (or good prose) unless you understand what it is you’re trying to say. So just because you’re a programmer doesn’t mean you get to forget everything you learned in English class. What you’re doing when programming is actually a lot more like writing essays than you think!

Comments also matter from a maintainability standpoint. I love Martin Fowler’s quote:

Any fool can write code that a computer can understand. Good programmers write code that humans can understand.

When you are writing code, you always have to think about your teammates or even the developer that will interact with your code 10 years from now.

Lastly, comments are a critical piece of your API. Joshua Bloch kicks off his item in Effective Java covering Javadoc comments with the line:

If an API is to be usable, it must be documented.

Think about all the times you’ve tried to integrate a 3rd party library that had limited or missing Javadocs. How frustrating was your experience? Writing quality Javadocs can make all the difference when it comes to the success of your APIs. Looking for a good example to emulate? Check out Guava or even the Javadocs for the JDK.

Inline Comments

Nothing can be quite so helpful as a well-placed comment. […] Nothing can be quite so damaging as an old crufty comment that propagates lies and misinformation.

Comments are not like Schindler’s List. They are not "pure good." Indeed, comments are, at best, a necessary evil. If our programming languages were expressive enough, or if we had the talent to subtly wield those languages to express our intent, we would not need comments very much – perhaps not at all.

The proper use of comments is to compensate for our failure to express ourself in code. Note that I used the word failure. I meant it. Comments are always failures. We must have them because we cannot always figure out how to express ourselves without them, but their use is not a cause for celebration.

So when you find yourself in a position where you need to write a comment, think it through and see whether there isn’t some way to turn the tables and express yourself in code. Every time you express yourself in code, you should pat yourself on the back. Every time you write a comment, you should grimace and feel the failure of your ability of expression.

When I first read this except from Clean Code, it struck me. It’s a pretty bold and almost harsh take, but I tend to agree with the author here. The passage continues:

Why am I so down on comments? Because they lie. Not always, and not intentionally, but too often. The older a comment is, and the farther away it is from the code it describes, the more likely it is to be just plain wrong. The reason is simple. Programmers can’t realistically maintain them.

Code changes and evolves. Chunks of it move from here to there. Those chunks bifurcate and reproduce and come together again to form chimeras. Unfortunately the comments don’t always follow them – can’t always follow them. And all too often the comments get separated from the code they describe and become orphaned blurbs of ever-decreasing accuracy.

I can think of so many examples when I’m reviewing code where code gets changed and a comment is left unchanged. Yes, the bug was fixed and the code functions properly, but now you are left with an obsolete comment that is bound to confuse somebody in the future. Maintaining comments can be extremely difficult, so let’s talk about a few ways to cut down on the number of inline comments to begin with.

Explain Yourself with Your Code

One of the best ways to reduce comments is actually pretty simple – writing more expressive code. By striving to write code that reads like prose, you’ll find that many comments become unnecessary!

Conditionals are a common case where comments can be eliminated with some simple refactoring. Consider this example – which would you prefer? This:

// check to see if the employee is eligible for full benefits
if ((employee.flags & HOURLY_FLAG) && (employee.age > 65))
{
    ...
}

or this:

if (employee.isElibigleForFullBenefits())
{
    ...
}

Not only does the second example make the client code cleaner by encapsulating the details of the benefits eligibility requirements, it also reads more like prose and eliminates the comment entirely. Win-win! IntelliJ has a built-in "extract method" refactoring shortcut (ctrl + alt + m) that can make this even easier to do.

Redundant Comments

A comment is redundant if it describes something that adequately describes itself. For example:

i++; // increment i

While this may seem like an extreme example, I’m sure everyone has seen cases like this where a comment is simply unnecessary. Comments should say things that the code cannot say for itself.

I’ve found that this pattern is especially common with beginner programmers. With the best of intentions they comment almost every line of code, thinking they are doing a great job of explaining themselves and writing maintainable code. How many times have you seen code like this?

// Display failure message
statusLabel.setText("Failure");

// Show retry button
retryButton.setVisible(true);

In reality, this code is written well enough to be understood without the comments. The comments only add clutter and can lead to the problems previously discussed.

Commented-Out Code

It makes me crazy to see stretches of code that are commented out. Who knows how old it is? Who knows whether or not it’s meaningful? Yet no one will delete it because everyone assumes someone else needs it or has plans for it.

That code sits there and rots, getting less and less relevant with every passing day. It calls functions that no longer exist. It uses variables whose names have changed. It follows conventions that are long obsolete. It pollutes the modules that contain it and distracts the people who try to read it. Commented-out code is an abomination.

When you see commented-out code, delete it! Don’t worry, the source code control system still remembers it. If anyone really needs it, he or she can go back and check out a previous version. Don’t suffer commented-out code to survive.

There’s not much more to be said on this topic – Clean Code has us covered. Nevertheless, be on the lookout for commented out code and be sure to delete it when you get the chance!

Javadoc Comments

As we mentioned before, Javadoc comments are the best, most effective way to document your API. The conventions for Javadoc comments are well established (see the Javadoc guide or Effective Java item 56), and you can set up Checkstyle to enforce several of them. In this post, I’m going to cover the most common violations of these conventions I see when reviewing code.

Methods

When writing Javadocs for methods, you want to say what the method does rather how it does its job. This is an application of the general principle that APIs should be free of implementation details, as they confuse users and inhibit the flexibility to involve. Some other key points to remember are:

  • The summary description should be a verb phrase (including any object) describing the action performed by the method
  • The text following an @param or @return tag should be a noun phrase
  • The text following an @throws tag should consist of the word "if," followed by a clause describing the conditions under which the exception is thrown
  • By convention, the phrase or clause following an @param, @return, or @throws tag is not terminated by a period

Good Example

/**
 * Returns the element at the specified position in this list.
 *
 * <p>This method is <i>not</i> guaranteed to run in constant
 * time. In some implementations it may run in time proportional
 * to the element position.
 *
 * @param  index index of element to return; must be
 *         non-negative and less than the size of this list
 * @return the element at the specified position in this list
 * @throws IndexOutOfBoundsException if the index is out of range
 *         ({@code index < 0 || index >= this.size()})
 */
E get(int index);

Notice the use of the words "this list" in the doc comment. By convention, the word "this" refers to the object on which a method is invoked when it is used in the doc comment for an instance method. This can be super helpful for writing clearer Javadocs for your methods and differentiating between static and instance methods. Conversely, be careful not to use the word "this" in the wrong context.

Verb Tense

Of all the problems with method Javadocs, this is absolutely the most common. You can even find violations of this in Android and other popular frameworks. Consider two correct examples:

  • ArrayList(int initialCapacity) – Constructs an empty list with the specified initial capacity.
  • Collection.size() – Returns the number of elements in this collection.

As shown in these examples, use the third person declarative tense ("returns the number") rather than the second person imperative ("return the number"). You can think of there being an implied "This method …" at the start of each method’s doc if it helps you out.

Exceptions

When it comes to exceptions, the biggest thing to remember is to document every exception that can be thrown by each method that you write. This goes for checked and unchecked exceptions as well as for abstract and concrete methods. For more on documenting exceptions, check out Effective Java item 74.

Thread Safety

Whether or not a class or method is thread-safe, its thread-safety level must be documented. Consider what Joshua Bloch has to say in Effective Java:

How a class behaves when its methods are used concurrently is an important part of its contract with its clients. If you fail to document this aspect of a class’s behavior, its users will be forced to make assumptions. If these assumptions are wrong, the resulting program may perform insufficient synchronization or excessive synchronization. In either case, serious errors may result.

Remember that thread-safety is not an all or nothing thing. Common cases of thread-safety levels include:

  • Immutable – Instances of this class appear constant. No external synchronization in necessary
    • Exs: String, Long, BigInteger
  • Unconditionally thread-safe – Instances of this class are mutable, but the class has sufficient internal synchronization that its instances can be used concurrently without the need for any external synchronization
    • Exs: AtomicLong, ConcurrentHashMap
  • Conditionally thread-safe – Like unconditionally thread-safe, except that some methods require external synchronization for safe concurrent use
    • Exs: Collections.synchronized wrappers (iterators require external synchronization)
  • Not thread-safe – Instances of this class are mutable. To use them concurrently, clients must surround each method invocation with external synchronization of the clients’ choosing
    • Exs: ArrayList, HashMap

For more on this topic, check out Effective Java item 82.

Classes and Fields

For classes, interfaces, and fields, the summary description should be a noun phrase describing the thing represented by an instance of the class or interface or by the field itself. Some good examples would be:

  • Instant – An instantaneous point on the time-line.
  • Math.PI – The double value that is closer than any other to pi, the ratio of the circumference of a circle to its diameter.

If you’re having trouble remembering the verb/noun phrase guidelines, just think about how in Object Oriented programming, our classes represent objects (nouns) while our methods represent actions (verbs) that we can take on those objects. If it’s still hard to write a good verb phrase for your method or a noun phrase for your class, it might be a sign that you are not thinking very object-oriented!

Enums

Enums can be an especially tricky case for documenting a class. The temptation can be to describe what the set of enum constants represent (and similarly to name the enum with a plural noun – but don’t do it!). Remember in the class summary you are describing the thing represented by an instance of the class. Consider this example from Effective Java.

/**
 * An instrument section of a symphony orchestra.
 */
public enum OrchestraSection
{
    /**
     * Woodwinds, such as flute, clarinet, and oboe.
     */
    WOODWIND,

    /**
     * Brass instruments, such as French horn and trumpet.
     */
    BRASS,

    /**
     * Percussion instruments, such as timpani and cymbals.
     */
    PERCUSSION,

    /**
     * Stringed instruments, such as violin and cello.
     */
    STRING
}

Be sure to document the constants along with the type and any public methods.

Final Note on Javadocs

If you ever have any doubts when it comes to Javadocs, the best thing to do is read the web pages generated by the Javadoc utility. While it sounds obvious, most developers never take the time to review what their Javadocs will look like to the outside world.

If you’ve never done it before, generating Javadocs is a lot easier than it sounds. Build systems like Gradle and Maven have built-in tasks that will create the Javadocs for your whole project to view in a web browser. To make things even simpler, IntelliJ recently added an in-editor Javadoc rendering setting. Super useful!

Conclusion

I hope this post has given you some useful tips on ways to step up your comment game. Maybe these are new ideas for you or just a good refresher on habits you already practice.

Leave a Reply