Writing Good Comments

I’ve been a software developer for nearly ten years now. I’ve read comments in nearly every language and code-base I’ve worked in, and written them in most. Over time, my opinion has shifted quite a bit on what makes a comment useful. This post is a reflection on that collective experience, and I hope will influence you in the comments you write.

Avoid Things that are Obvious

Let’s start with the most obvious thing: what shouldn’t you leave as a comment in source code? Consider this example in C#:

public int Add(int x, int y) {
    // Adds x to y and returns the result to the caller
    return x + y;
}

This comment is completely useless. Sorry, not sorry. It doesn’t tell you anything that is not immediately clear from code. It would be more helpful to the reader if it explained something about edge cases, such as:

public int Add(int x, int y) {
    // Adds x to y and returns the result to the caller. If the operation
    // results in underflow (where x + y < 0) - due to the sum of x and y
    // being greater than 2^31-1, then the final result will be less than
    // zero.
    return x + y;
}

That comment is more useful, because it deals specifically with an edge-case (also referred to as a “corner case” in some circles), and tells you what to expect in such a circumstance.

While this sort of comment isn’t very useful for the reader of the method, it would make for a fabulous documentation comment. If you feel compelled to author these sorts of works in your source code, then you should also compel yourself to learn the appropriate doc-string syntax for producing it. In CSharp, it would look like this:

/// <summary>
/// Adds the <paramref name="x" /> parameter to the <paramref name="y" /> parameter
/// and returns the result to the caller. If the operation results in underflow
/// (where x + y < 0) - due to the sum of x and y being greater than 2^31-1,
/// then the final result will be less than zero.
/// </summary>
public int Add(int x, int y) {
    return x + y;
}

The C# compiler will take XML-style comments and produce a documentation file. Every development environment that’s worth using will show this information in a tool-tip if the user hovers their mouse over the method. It makes clear what the code is doing, and the reader can likely deduce the entire method implementation based on the string. Someone testing the code could even deduce whether or not the switches passed to the compiler regarding arithmetic overflow changed for a particular build based on this comment and observed program behavior.

In summary, avoid writing things that are obvious. If you must write things that are obvious, do it as a documentation comment that will surface itself as member documentation, and try to be thorough about edge cases.

Idea Path Comments

I’ve found that many beginners will write what I refer to as “idea path” comments. They’ll start writing their code, and construct a sort of journey for them to follow. It might look something like this:

public int Minimum(int[] values) {
    // 1. Assign an arbitrary minimum value
    // 2. Loop over all integers in the `values` array and compare
    //    to the current minimum. If the "next" value is less than
    //    the "current" value, track it as the minimum.
    // 3. Return the minimum.
}

They will then write the method or function:

public int Minimum(int[] values) {
    // 1. Assign an arbitrary minimum value
    var min = values[0];

    // 2. Loop over all integers in the `values` array and compare
    //    to the current minimum. If the "next" value is less than
    //    the "current" value, track it as the minimum.
    for (var i = 1; i < values.Length; i++) {
        var current = values[i];
        if (current < min) {
            min = current;
        }
    }

    // 3. Return the minimum.
    return min;
}

This seems fine, but it just occupies space. It certainly doesn’t provide any additional information. In fact, writing the comment took more time than just writing the code. In my opinion, this is harmful to productivity, both for the reader, and the writer. As a maintainer, if I see this sort of thing, I will also tend to feel a responsibility to check that the code and the comments conform well to one another, since the comments serve as a form of specification; if the two do not match, it just adds cognitive load. This sort of commentary has no place in a professional code-base, in my opinion. If you are the idea-path type of programmer, you may want to consider an alternative style of code, where you simply stub out a bunch of method helpers. That would look like this:

public int Minimum(int[] values) {
    var min = GetInitialMinimum(values);
    FindMinimum(values, ref min);
    return min;
}

private static int GetInitialMinimum(int[] values) {
    return values[0];
}

private static int FindMinimum(int[] values, ref min) {
    for (var i = 1; i < values.Length; i++) {
        var current = values[i];
        if (current < min) {
            min = current;
        }
    }
}

For a method as simple as Minimum, this seems like overkill, but I’ve seen this used to great success in more complex forms of code, including parsers and complex orchestrators. Each method is so small and deliberate that the reader can focus on understanding it completely. One potential drawback of this approach is that it can make understanding more difficult in the aggregate, though I suspect for any reasonably complicated piece of code, the energy expenditure will be similar regardless of approach.

With all that said, that does not mean that the idea-path style shouldn’t be used; if you are writing code that is intended to be instructional, then this sort of commentary can be a very useful tool for explaining an algorithm to your reader. I seem to recall seeing things like this in Sedgewick’s Algorithms text. When the goal of the code is to be instructional, this can be a tremendous aid to your audience. In such a scenario, it’s also worthwhile to be complete in your commentary.

Task Comments

I’m sure we’ve all seen the // TODO: comment at least once. This sort of call-to-action and request to improve something is a great way to signal that some future work needs to be performed, but is unclear when it can be done. I generally appreciate this sort of comment. It’s often difficult to refactor and improve things, but the person most likely to be prepared to conduct such an activity is usually the person that most recently read the code. If that’s you, and you think there’s opportunity to make an improvement, a // TODO: comment that explains what changes you want to make and why can be a huge help. I’d personally avoid leaving initials, since that can be derived from your source control history, but I’ve seen it done and have done it myself.

Explanatory Comments

Moving past the first two categories, we arrive at what I refer to as “explanatory comments.” These tell you something about the overall context that the code was being written for that will never be apparent by reading the source code directly. They will explain things about observed behavior in a library, a trade-off made by the implementer, or something similar. These are also sometimes called “why” comments: they don’t tell you what the code is doing, as it should be immediately obvious under inspection what’s happening, but why it’s doing it. These are often the best comments you’ll ever find in a code-base, and can be a tremendous source of learning. Here’s one example of such a comment:

public async Task<Order> GetOrderStatus(string orderId, CancellationToken cancellationToken) {
    // This creates a linked cancellation token. The first comes from the caller via the
    // `cancellationToken` parameter, and specifies whether or not _they_ have been
    // cancelled (such as, by the program shutting down). The second - `limit` - is a
    // limit *we* are imposing on the overall operation. We want this method to cancel
    // if *either* of these is observed.
    var limit = new CancellationTokenSource(TimeSpan.FromSeconds(5));
    using var cts = CancellationTokenSource.CreateLinkedToken(cancellationToken, limit);

    try {
        return ordersClient.GetAsync(orderId, cts.Token).ConfigureAwait(false);
    }
    catch (OperationCanceledException) {
        if (!cancellationToken.IsCancellationRequested) {
            // We don't want to propagate an OperationCanceledException here, because
            // calling code may "believe" that the `cancellationToken` they passed
            // was signaled. Instead, we'll throw a specific exception indicating that
            // the operation timed out. This hides details about us using an additional
            // cancellationToken inside our method.
            throw new OrderQueryTimeoutException("The order query timed out after 5 seconds.");
        }
    }
}

I have seen and written many comments like these. I found a recent nugget where I had to document why I created an STA thread in a Windows Forms application. The entire use-case was bizarre, because that’s supposed to be taken care of for you, and yet, we needed to do it ourselves for whatever reason. The overall comment explained why it was needed, and also left a closing // TODO: figure out why we're not started correctly comment at the end, because creation of that particular object was supposed to occur automatically.

An Alternative to Commenting Source Code

Code comments are useful, but one thing you should always keep in the front of your mind is that your system should have documentation somewhere. Design docs, specifications, reference material, release notes notes, and other such artifacts, are also good candidates for where to put information that you may be compelled to include in your source code. I personally write very detailed commit messages. Those commit messages eventually help with creation of release notes. A comment is essentially a form of documentation. You should consider whether it is most appropriate to include in source code, or if you should put that content elsewhere. With commits in particular, I generally follow the same guidelines that have been espoused in the dotnet/runtime repo, which they referenced under A Note about Git Commit Messages.

Conclusion

At the end of the day, writing comments in source code is akin to any other form of writing: know your reader, and be deliberate about what you choose to include. If you work on a team, ask yourself if the comment follows team norms. If you’re contributing to an existing code base, take some time to read the existing code and understand what the system looks like. Ask if there is any specific standard the team follows. Comments can provide a lot of value. Do your best to make sure you’re writing things that are useful to future readers, and try to minimize the amount of information they need to sift through in understanding your code.