Fighting Evil in Your Code: Comments on Comments

One of the most glib generalisations you can make about development work is to say that code should be liberally commented, or conversely that it should never be commented. As always, the truth is more complicated. There are many different types of comment and some types are best treated firmly with the delete key, where others are to be cherished and maintained assiduously. Even though it is hard to find two developers who agree on the topic of commenting, Michael Sorens warily sketches out the issues and the battleground.

Like the temperature of coffee, or the value of gold-plated A/V connectors, the debate about whether to comment your code has inexorably become more and more contentious. Commenting code is on the generally-recognized list of code smells. But unlike most of the others on the list, comments are not always bad. Generally speaking, you should strive to minimize comments and let the code speak for itself, but it is just as important to include comments when the code cannot.

First, I just have to quote Erik Dietrich in his recent post Is There a Correct Way to Comment Your Code? because I got a great chuckle out of the way he phrased this:

“Before we get to my take, though, let’s go watch programmers do what we love to do on subjects like this: argue angrily.”

That nested link is to a StackOverflow post on whether one should comment or not. In there you’ll find strong opinions on both sides, usually because each person focuses on the point that is most beneficial—or most annoying—to them. In fact, I claim that it is meaningless to ask whether comments are good or bad without considering the context. In this article, I break down commenting into its constituent categories and consider the degree of evil of each in turn. Yes, comments are evil. And, yes, comments are good. Your task is to recognize and combat the evil which, once you finish reading, you will readily be able to do (even without the help of the estimable brothers Winchester).

What’s Wrong with Comments

The problems with comments are many and varied:

  • Over time, and not intentionally, comments can lie, so can lead to misinterpreting the code if you happen to believe the comment.
  • Writing comments takes time, more so if you choose a commenting style that requires a lot of time to look pretty, so strive for simplicity.
  • Comments make a file longer and can often introduce unnecessary clutter, thus requiring more time to read. Steve Smith, in When to Comment Your Code, reminded me of a very relevant quote from that classic tome on writing, The Elements of Style, and it applies just as validly to code: “Omit needless words. Vigorous writing is concise. A sentence should contain no unnecessary words, a paragraph no unnecessary sentences, for the same reason that a drawing should have no unnecessary lines and a machine no unnecessary parts.”
  • Comments often attempt to explain what or how, which tends to merely repeat the code; comments should always address the why.
  • In maintaining code you also have to maintain any associated comments too, thus requiring more effort.
  • Writing comments that are clear rather than cryptic is hard. I recent comment (pun intended) I read on this was enlightening: if the code was written in a confusing manner why would you expect the code author to suddenly be able to write comments about it clearly?

As I have said here, and you may have seen elsewhere, comments frequently lie. What does that really mean? Here’s an example from Dietrich’s article along with his commentary:

// Returns x + y or, if x or y is less than zero, throws an exception
public int Add(int x, int y)
{
    return x + y;
}

What happened here? When you put on your code archaeology hat, you’ll probably conclude that a guard condition once existed. Someone deleted that guard condition but didn’t think to update the now-nonsensical comment. Oops.

Oops, indeed! Comments, unlike code, are not checked at edit time (unless you’re writing doc-comments, discussed later) or at compile time or at runtime. You cannot lint your comments. You cannot unit test your comments. In other words, there are no safeguards. Thus, whether you are dealing with good comments or evil comments, you should strive to minimize them so they do not become stale or downright misleading, as in the above example.

Many developers do not like to write documentation of any sort, and that includes comments. (For a good list of excuses reasons, see Why programmers don’t comment their code. But commenting too little (or not at all) is just as bad as commenting too much. As I and many others have stated, comments are often necessary, but try to keep them to a minimum.

The 9 Categories of Comments

After reading everything I could find on comments, and from my own experience as a developer for 20+ years, I developed the taxonomy of comments given in the table below. The sections that follow provide details of each category in this table.

It is interesting to note that, if you consider the entire table as a whole, comments are roughly 50% good and 50% evil.

Category

Good or Evil?

Why?

Explaining complicated code (“what” or “how”)

95% Evil

  • Typically indicative of code that should be rewritten, obviating the need for such comments.
  • Clutters the code unnecessarily.
  • Increases maintenance costs.
  • Becomes inaccurate and worse, misleading, over time.

Repeating the code in different words

100% Evil

  • Adds no value to the reader.
  • Violates DRY (Don’t Repeat Yourself).
  • Clutters the code unnecessarily.
  • Increases maintenance costs.
  • Becomes inaccurate and worse, misleading, over time.

Markers (e.g. “to do” tags)

25% Evil

  • When used consistently allows easy searching of unfinished items required for release.
  • Make sure that those associated with items required for release are removed before release; OK to keep others around.

Explaining code rationale (the”why” of the code)

0% Evil

  • Summary or intent comments help show the bigger picture.
  • Use to explain why choices were made or how broken conventions are necessary for performance or compatibility, etc.

Resource inclusion

0% Evil

  • Describe particulars that cannot be expressed by code, including references, legal notices, provenance, etc.

Witticisms (self-indulgent comments)

75% Evil

  • Cute at the moment you write it but chances are even you (let alone a colleague) would not understand it a week later.
  • Sometimes, though, it is fun to have a chuckle when you read some code.

API doc-comments

25% Evil

  • Good when well-written; providing useful information.
  • Not so good when providing no useful information.

Commented-out code

100% Evil

  • Useful in the heat of coding for debugging or experimentation, but should be used as a temporary measure only.

Syntax Extension

50% Evil

  • Typically frowned upon but necessary on occasion for backwards compatibility

Explaining Complicated Code

This category of comment is possibly the most prevalent among the evil pool, and takes work to cleanse. Martin Fowler, in Refactoring: Improving the Design of Existing Code, states the treatment of such comments eloquently: “When you feel the need to write a comment, first try to refactor the code so that any comment becomes superfluous.” Kernighan and Plauger, in Elements of Programming Style, make the point even more succinctly: “Don’t document bad code – rewrite it.”

Follow the code block shown below (in JavaScript) through successive refactorings to strip out all the comments and, in the process, make the code cleaner, easier to follow, and thus easier to maintain.

function showNotification(timeout) {

    // reveal notification
    var n = $('#notification'); // get DOM node; '#' selects an ID attribute
    n.slideDown(
        200, // transition time in milliseconds
        function noop() { }
    );

    // set timer to hide notification unless user directs otherwise
    timeout = timeout || 5000; // default to 5 seconds (in milliseconds)
    if (timeout !== -1) { // user value of -1 indicates to skip
        $timeout(
            function () { hideNotification(); },
            timeout
        );
    }
}

As each rule is applied, I highlight in red the portion of the code that has been updated. Compare to the prior code sample to see the “before” vs. the “after”. Here is the first one:

Rule 1: Rename variables to be meaningful

Here the comment has become shorter because the variable name now indicates it is a node. (The rest of the comment will go away with application of rule 3 below.)

function showNotification(timeout) {

    // reveal notification
    var notifyNode = $('#notification'); // '#' selects an ID attribute
    notifyNode.slideDown(
        200, // transition time in milliseconds
        function noop() { }
    );

    // set timer to hide notification unless user directs otherwise
    timeout = timeout || 5000; // default to 5 seconds (in milliseconds)
    if (timeout !== -1) { // user value of -1 indicates to skip
        $timeout(
            function () { hideNotification(); },
            timeout
        );
    }
}

Rule 2: Extract inline code to separate, named functions

function showNotification(timeout) {
    revealNotification();
    timeout = timeout || 5000; // default to 5 seconds (in milliseconds)
    if (timeout !== -1) { // user value of -1 indicates to skip
        setNotificationExpiration(timeout);
    }
}

function revealNotification() {
    var notifyNode = $('#notification'); // '#' selects an ID attribute
    notifyNode.slideDown(
        200, // transition time in milliseconds
        function noop() { }
    );
}

function setNotificationExpiration(timeout) {
    $timeout(
        function () { hideNotification(); },
        timeout
    );
}

Rule 3: Replace magic values with named constants

Several applications of this rule are done simultaneously here, both for integers and strings.

function showNotification(timeout) {
    revealNotification();
    var defaultTimeout = 5000; // milliseconds
    timeout = timeout || defaultTimeout;
    if (timeout !== -1) { // user value of -1 indicates to skip
        setNotificationExpiration(timeout);
    }
}

function revealNotification() {
    var idAttributeSelector = '#';
    var notificationBarId = 'notification';
    var notifyNode = $(idAttributeSelector + notificationBarId);
    var transitionTime = 200; // milliseconds
    notifyNode.slideDown(
        transitionTime,
        function noop() { }
    );
}

function setNotificationExpiration(timeout) {
    $timeout(
        function () { hideNotification(); },
        timeout
    );
}

Rule 4: Include units in variable names to disambiguate numbers

function showNotification(timeout) {
    revealNotification();
    var defaultTimeoutInMilliseconds = 5000;
    timeout = timeout || defaultTimeoutInMilliseconds;
    if (timeout !== -1) { // user value of -1 indicates to skip
        setNotificationExpiration(timeout);
    }
}

function revealNotification() {
    var idAttributeSelector = '#';
    var notificationBarId = 'notification';
    var notifyNode = $(idAttributeSelector + notificationBarId);
    var transitionTimeInMilliseconds = 200;
    notifyNode.slideDown(
        transitionTimeInMilliseconds,
        function noop() { }
    );
}

function setNotificationExpiration(timeout) {
    $timeout(
        function () { hideNotification(); },
        timeout
    );
}

Rule 5: Convert expressions to named values

function showNotification(timeout) {
    revealNotification();
    var defaultTimeoutInMilliseconds = 5000;
    timeout = timeout || defaultTimeoutInMilliseconds;
    var userRequestsSkip = timeout === -1;
    if (!userRequestsSkip) {
        setNotificationExpiration(timeout);
    }
}

function revealNotification() {
    var idAttributeSelector = '#';
    var notificationBarId = 'notification';
    var notifyNode = $(idAttributeSelector + notificationBarId);
    var transitionTimeInMilliseconds = 200;
    notifyNode.slideDown(
        transitionTimeInMilliseconds,
        function noop() { }
    );
}

function setNotificationExpiration(timeout) {
    $timeout(
        function () { hideNotification(); },
        timeout
    );
}

Those 5 rules cover the bulk of the refactorings necessary to eradicate harmful comments. But there are likely others that you have come across or developed yourself, so feel free to leave a comment at the end of the article to add to this list!

There is one subcategory of comments in this group that cannot be effectively expunged by rewriting: explaining regular expressions. If you use regular expressions—and that’s a big “if” I understand—you should be aware that you can reduce the “arcane quotient” significantly by commenting them.

Which looks more understandable and maintainable? Here’s a relatively simple regex without commenting:

  (19|20)\d\d[- /.](0[1-9]|1[012])[- /.](0[1-9]|[12][0-9]|3[01])

And here it is again with commenting:

(19|20)\d\d                # year (group 1)
[- /.]                     # separator
(0[1-9]|1[012])            # month (group 2)
[- /.]                     # separator
(0[1-9]|[12][0-9]|3[01])   # day (group 3)

To include comments within a regular expression, you need support in your language for free-spacing regular expressions—which is available for most modern regex engines—and that typically includes support for embedded comments beginning with the octothorp (#) character. See Free-Spacing Regular Expressions for more details on the construct in general; and Miscellaneous Constructs in Regular Expressions for .NET support in particular.

What to do about comments that explain complicated code?
Fix the code! Rewrite/refactor to eliminate the need for these comments.

Repeating the code in different words

This example (In Java) is taken from Steve McConnell’s Code Complete, second edition, section 32.4. While not always black and white, a good portion of the time such comments really do state the obvious, as this example illustrates. Removing them does not degrade the code at all; indeed, it makes the code cleaner, simpler, and easier to follow.

BEFORE:

// set product to "base" 
product = base; 
 
// loop from 2 to "num" 
for ( int i = 2; i >= num; i++ ) { 
   // multiply "base" by "product"  
   product = product * base; 
}

AFTER:

product = base; 
 
for ( int i = 2; i <= num; i++ ) { 
   product = product * base; 
}

What to do about comments that repeat the code?
Remove them!

Markers

Occasionally as you write code you will choose to skip over some detail for the moment, fully intending to come back later and correct or expand upon it. A common convention is to use a special prefix within a comment to mark such places, be it “TBD” or “FIXME” or “!!!!!!!!!!!”. In Visual Studio, if you begin comments with the label “TODO” or “UNDONE”, those tagged comments are conveniently collected into the Task List window for you. For this C# sample…

…the Task List shows those so- designated comments (by project, file, and line number):

If you prefer your own term (see Wikipedia for some further examples), you can configure them in the Options panel. Here’s what you get by default in Visual Studio 2015:

Generally speaking, these marker comments identify technical debt. They may often be small bits—things that are inconsequential or non-time sensitive. But there may be some critical bits as well; things that absolutely must be tended to before a release.

What to do about marker comments?
Ensure that remaining ones are acceptable for committing and/or releasing.

Explaining code rationale

Comments of this ilk are not only useful but often critical to ensure performant, reliable, and/or maintainable (non-fragile) code. They also serve—as several sources including Wikipedia point out—to explain why a chunk of code does not seem to fit conventions or best practices.

A few examples from Ron Swartzendruber in this post:

  // We use Algorithm X here because of peculiarities in our dataset. See person Y for details.
  # This function should not be renamed because it is called from legacy system Z which we can't update.
  // Extra checks needed here until we solve Bug 1234.

In the StackOverflow post mentioned earlier (discussing the merits of commenting or not), one particularly poignant remark states, in effect, “Great programmers tell you why a particular implementation was chosen [but]master programmers [also] tell you why other implementations were not chosen.” This example from Jeff Atwood’s column Code Tells You How, Comments Tell You Why illuminates that point, explaining both why and why not:

  /* A binary search turned out to be slower than the Boyer-Moore algorithm for the data sets of interest, thus we have used the more complex, but faster method even though this problem does not at first seem amenable to a string search technique. */

(Want to see lots more examples? Search through your code base for the word “because” and you are likely to find quite a number of such important comments. It is not a universal, by any means, but provides a great chance of finding some.)

As part of explaining or summarizing the intent of a file or a method, you might want to include a simple flowchart or other diagram rendered in plain text. Sites like Asciiflow let you create diagrams in plain text; also see this StackExchange post for assorted downloadable tools that work similarly and produce images like this:

That’s not to take the place of external, polished documentation, but often having internal documentation, close to the code, makes it more useful.

What to do about comments that explain code rationale?
Keep them. Nurture them. Shower them with kindness. They will repay you in spades.

Resource Inclusion

Code Complete calls this category “comments beyond the code”. This includes:

  • Copyright notice
  • License information
/* ***** BEGIN LICENSE BLOCK *****
 * Version: MPL 1.1
 *
 * The contents of this file are subject to the Mozilla Public License Version
 * 1.1 (the "License"); you may not use this file except in compliance with
 * the License. You may obtain a copy of the License at http://www.mozilla.org/MPL/
 * . . .
 *
 * ***** END LICENSE BLOCK *****
 */
  • Logos done in plain text (a form of “ASCII art”), as this one from AsciiWorld.com:

  • URL reference to relevant material, e.g. a link to where an algorithm came from. If I am using something verbatim I just say “from” but often I need to customize something I’ve found, so I preface with “adapted from” just to give the reader a bit more information.
// Adapted from http://www.csharp-examples.net/restart-windows-service/
public static class WindowsServiceController
{ . . . }
  • Lacking a specific reference, often just naming something can be helpful, allowing the reader to research the topic further if desired; this example is adapted from Code Complete:
function squareroot(num) {
    // Newton-Raphson approximation to determine square root
    Var r = num / 2; 
    while ( abs( r - (num/r) ) > TOLERANCE ) { 
       r = 0.5 * ( r + (num/r) ); 
    }
    return r;
} 
  • File headers specifying file metadata, perhaps including version control key fields, the date the file was created, and what project it belongs to, as in this example showing Subversion key fields in the first line:
/*
 * ==============================================================*
 * ID       $Id: FileMask.cs 971 2015-09-30 16:09:30Z ms $      *
 * Created  2015-08-19                                          *
 * Project  http://cleancode.sourceforge.net/                   *
 * ==============================================================*
 *
 */

What to do about resource comments?
These are typically necessary either for legal reasons or to follow company conventions or to lead a reader to original source material.

Witticisms

Code Complete refers to this category as “self-indulgent comments”; Wikipedia refers to it as “stress relief”. This fine example, in assembly language, comes verbatim from Code Complete, section 32.5:

Many years ago, I heard the story of a maintenance programmer who was called out of bed to fix a malfunctioning program. The program’s author had left the company and couldn’t be reached. The maintenance programmer hadn’t worked on the program before, and after examining the documentation carefully, he found only one comment. It looked like this:

  MOV AX, 723h    ; R. I. P. L. V. B.

After working with the program through the night and puzzling over the comment, the programmer made a successful patch and went home to bed. Months later, he met the program’s author at a conference and found out that the comment stood for “Rest in peace, Ludwig van Beethoven.” Beethoven died in 1827 (decimal), which is 723 (hexadecimal). The fact that 723h was needed in that spot had nothing to do with the comment. Aaarrrrghhhhh!

That one is rather esoteric, but there are many that are more… enlightening. Here is a sampling:

  // Sorry for what you are about to see 

–John Vester’s Code Commenting Patterns

  /* You are not expected to understand this */

–Dennis Ritchie’s—yes, that Dennis Ritchie!— Odd Comments and Strange Doings in Unix

/**
* For the brave souls who get this far: You are the chosen ones,
* the valiant knights of programming who toil away, without rest,
* fixing our most awful code. To you, true saviors, kings of men,
* I say this: never gonna give you up, never gonna let you down,
* never gonna run around and desert you. Never gonna make you cry,
* never gonna say goodbye. Never gonna tell a lie and hurt you.
*/

–Jens Roland’s StackOverflow answer to What is the best comment in source code you have ever encountered?

And a few others found amidst the myriad of other responses to the same StackOverflow question:

  /* I don't know how you can ever get here so I'll have to fix it later */
  // This is a kind of magic...
  -- Change Log:  Not needed. The code is perfect 'cause I wrote it.
  -- If you change it, it will break.
  // I don't know why. Just move on.
  // This should fix something that should never happen

What to do about witty comments?
It is with some measure of reluctance that I must say that many of them should be struck out. But for those neither in bad taste nor obscure, you might want to leave them be just to bring a smile to some future reader’s face.

API Doc-Comments

Many, many applications today are all about creating a library with a published API for your customers to utilize said library. Such APIs are typically generated programmatically from documentation comments (doc-comments) embedded in your source code. In the .NET world, for example, you need to enable documentation generation for each project you are concerned with, under the project’s Build properties, as shown.

The C# compiler then processes doc-comments for those projects, creating an intermediary XML file that can be used by tools like Sandcastle to create final, user-facing documentation; see XML Documentation Comments for further details. In the Java world, Javadoc works in very much the same way on embedded doc-comments.

Doc-comments should be applied to every public member of your class, i.e. all those exposed in your public API. You are not restricted to just the public members, but in the spirit of keeping comments to a minimum, my suggestion is to limit yourself to those. Even so, there are good doc-comments and bad doc-comments. For the FitsOneOfMultipleMasks method below, here’s a bad doc-comment.

/// <summary>
/// Determines whether the given file name fits one of multiple masks.
/// </summary>
/// <param name="fileName">Name of the file</param>
/// <returns>Boolean</returns>
public bool FitsOneOfMultipleMasks(string fileName)
{ . . . }

Though it includes the requisite summary, parameter, and return value fields, this is really just a disguised instance of the comment category “repeating the code in different words”. These comments do not add anything of value. They do not help the user of your API determine how to use the method any more than reading the method signature already provided. Also, depending on the documentation generator, it might even know how to generate such comments by itself were they omitted here!

Here is the same method with a bit more useful doc-comments. The summary could arguably be expanded more, but at least by using different words it gives the reader another perspective. Together with the detail added to the input and output, it provides the reader with some useful information.

/// <summary>
/// Determines whether the given file name matches
/// one or more of the specified masks.
/// </summary>
///
/// <param name="fileName">Name of the file.
/// Multiple masks may be included (in the <see cref="Mask"/> property)
/// by separating them with lines, commas, spaces, or vertical bars.</param>
///
/// <returns>Boolean indicating whether the file name matches
/// one or more masks.</returns>
public bool FitsOneOfMultipleMasks(string fileName)
{ . . . }

What to do about doc- comments?
Make them meaningful by being aware of your audience, likely someone who, unlike you, is not intimately familiar with the code.

Commented-out Code

Most IDEs make it easy to comment out an arbitrary block of code, usually with a single keystroke. This is quite helpful as you are probing, experimenting, and otherwise trying out your ideas as you mold and refine your code. But once you have solidified your algorithms any leftover commented out code blocks should be removed; they have no business being in production code. If you are met with resistance to this—often put forth with contrition with “But we might need it again someday.” —your quick response should just be “that is what source control is for!” Indeed, any project using source control—which should of course be all projects containing at least one line of code(!)—has no need of such baggage comments. You can always go back to a prior version of your code to retrieve lines that have been deleted.

What to do about commented-out code?
Remove them!

Syntax Extension

This embeds pseudo-code constructs within regular comments so that they are largely ignored, except by a processor (or pre-processor) specifically looking for them. A widely known example of this is conditional comments from Internet Explorer, versions 5 through 9. You could, for example, write this to include a specific CSS file for version 8 of Internet Explorer only:

<!--[if IE 8]>
<link href="ie8only.css" rel="stylesheet">
<![endif]-->

Though support for such conditional comments was removed from IE version 10 and above, if you really need it you can simply tell a browser to use IE 9 emulation with this header:

  <meta http-equiv="X-UA-Compatible" content="IE=EmulateIE9">

That works at least for IE 10 and IE 11, according to this post. Conditional comments are one type of syntax extension. I am not aware of other uses of conditional comments (other than for an application I created a couple decades ago to allow a single file to support multiple versions of Pascal simultaneously. 🙂

Syntax extension, more generally, implements the Hot Comments pattern, allowing a way to extend a non-extensible format. You have already seen another use of this above with doc-comments. Both Java’s and C#’s doc-comments make use of this “Hot Comments” pattern.

What to do about syntax extension comments?
Your legacy system may require them, or you may have other logistical reasons for needing them. Either way, keep them as long as you need them.

Conclusion

The next time someone wants to convince you that comments are absolutely required, or conversely, that comments should never be used at all, you can now say, “Whoa!” (That’s American shorthand for “I must stop you right there, good sir or madam, as there are points you should consider before making such a hasty generalization.”) And you continue, “To which of the 9 types of comments do you refer?” Then you can wax eloquently on whether to use or not use the type of comment under discussion.