Comments on Comments on Comments

Tuesday Feb 20th 2007 by Jeff Langr
Share:

Another view on a long debated issue: comments within code.

In the article "In Praise of the Lowly Comment," Mike Gunderloy presents a good case for the sorts of comments that are appropriate and necessary. I agree with many of Mr Gunderloy's points, including one particularly important one, that "software [development] is hard." We also both are skeptical of the notion of self-documenting code.

Gunderloy summarizes four types of comments that are worthwhile for particularly difficult sections of code:

  • "Todo," or placeholder, comments
  • Summary comments
  • "Why," or intent, comments
  • Explanatory comments

I think these are worthwhile categorizations, and I'll add my, uh, comments to Gunderloy's thoughts.

Placeholder Comments

Placeholder comments are great reminders of things that need to be done. I use them occasionally. However, as Gunderloy mentions, you want to review these prior to shipping, to ensure that no embarrassing comments remain. My stance on placeholder comments is a bit more stringent: You don't want them to exist past an iteration or release boundary.

Placeholder comments are much like warnings and rabbits. As soon as you have a couple or more that stick around, they rapidly proliferate. This is the "broken window syndrome." Unfortunately, most teams aren't good at doing anything about such comments, so they often stick around forever. I've been embarrassed by "TODO" comments of my own that lay dormant in code for over a year.

The important thing is that developers get in the habit of looking for and actually fixing these hopefully small problems. First, as Gunderloy suggests, developers should use standard tag names (such as Eclipse's TODO) for such ephemeral comments. That will at least allow their regular and immediate perusal. Second, developers should have the attitude that placeholders represent incomplete work, and must be resolved by release or iteration end. At that time, any remaining placeholder comments are removed. Bigger "todo's" that can't be resolved in time get represented on some sort of external task list (which elevates their visibility).

Summary Comments

I've found summary comments to be occasionally valuable in explaining code. However, I tend to write summaries for classes, not methods, because there's usually a better way to represent code chunks within a method. Gunderloy mentions, "the more procedural the code gets," the more likely it needs a comment. My reaction is to refactor code to the point where it isn't so procedural (I'm talking about object-oriented code here).

The code in Listing 1, from a library system example codebase, is not atypical for most of the hundreds of systems I've encountered. Most developers would consider this code and its associated comments as perfectly acceptable. When teaching test-driven development, however, I have students work at improving the checkin method by using basic refactoring techniques.

My term for summary comments appearing within a method is "guiding comments:" They supposedly help you step through the body of a longer method. To me, they are beacons that often suggest application of the Extract Method refactoring.

Most students end up with a checkIn method similar to that shown in Listing 2. Each of the extracted methods, not shown, is now similarly short (most of them can be further cleaned up, and some could be moved to more appropriate classes). The code in checkIn now provides a readable, concise summary of a check-in policy. The willingness to extract methods and to freely rename variable and method names quickly turned a difficult method into a comprehensible one.

Listing 1: Difficult code

public void checkIn(String barCode, Date date, Branch branch) {
  Holding hld = findBarCode(barCode);

  // set the holding to returned status
  List holdings = null;
  hld.checkIn(date, branch);

  Holding patHld = null;

  // locate the patron with the checked out book
  Patron f = null;
  Patron p = null;
  for (Iterator it = getPatrons().iterator(); it.hasNext();) {
     p = (Patron)it.next();
     holdings = p.holdings();
     for (Iterator it1 = holdings.iterator(); it1.hasNext();) {
        patHld = (Holding)it1.next();
        if (hld.getBook().equals(patHld.getBook()))
           f = p;
     }
  }

  // remove the book from the patron
  f.remove(hld);

  // check for late returns
  boolean isLate = false;
  Calendar c = Calendar.getInstance();
  c.setTime(hld.dateDue());
  int d = Calendar.DAY_OF_YEAR;

  // check for last day in year
  if (c.get(d) > c
        .getActualMaximum(d)) {
     c.set(d, 1);
     c.set(Calendar.YEAR, c.get(Calendar.YEAR) + 1);
  }

  if (hld.dateLastCheckedIn().after(c.getTime()))    // is it late?
     isLate = true;

  if (isLate) {
     int daysLate = 1;    // calculate # of days past due
     switch (hld.getBook().getType()) {
        case Book.TYPE_BOOK:
           f.addFine(Book.BOOK_DAILY_FINE * daysLate);
           break;
        case Book.TYPE_MOVIE:
           int fine = Math.min(1000, 100 + Book.MOVIE_DAILY_FINE *
                               daysLate);
           f.addFine(fine);
           break;
        case Book.TYPE_NEW_RELEASE:
           f.addFine(Book.NEW_RELEASE_DAILY_FINE * daysLate);
           break;
     }
  }
}

Listing 2: Clearly stated policy

public void checkIn(String barCode, Date date, Branch branch) {
  Holding holding = findHolding(barCode);
  holding.checkIn(date, branch);
  Patron patron = findPatron(holding);
  patron.remove(holding);

  if (isLate(holding))
     addFines(patron, holding);
}

Intent Comments

Intent comments, as Gunderloy explains, provide the answer to "why?" I find myself needing these sorts of most frequently. "This hard-coded implementation of ln provides optimal performance against a specific range of data."

Some intent comments can be supplanted with unit tests. I practice test-driven development (TDD), and as such, I seek to craft my unit tests so they too are highly readable. Done well, TDD can produce test methods that read as examples of how to use a class. Done better, these examples can start to act as a form of specification. When interacting with a well-written TDD system, I use the tests as my first understanding of the code. They help me navigate and understand the capabilities of each of its classes.

Rocket-Science Comments

I too have found that there are certain things that will remain difficult in code, no matter how much you refactor. However, I've also found that these difficult things represent a small, small fraction of every system. Further, I'm usually able to at least get portions of the complexity under control. Most coding needs in most systems is pretty boring, simple stuff.

Final "Commentary"

Comments do not come without a cost. They take time to enter. Comments must be maintained in synch with the code. Often, they are not maintained, in which case they become useless at best, misleading at worst. They also can detract from the readability of code if they are excessive or redundant.

How do I really know if my code needs a comment? A pair generally knows the right answer. Pairing helps dramatically by ensuring that the code needs fewer comments through refactoring. In lieu of a pair, another great technique I've employed is to have someone spend a minute or two trying to paraphrase my code (test code too).

I no longer seek to defend comments. Instead, I view most comments as opportunities to improve upon the code and tests. Certainly, there will always be the rare exception, the complex code that cries out for a comment. But that's not an excuse to not try to first clean up the code. I look at every method with a critical eye, and try to find a way to make it shorter or clearer through elimination of comments. If it's still unclear, only then do I add a comment.

Maintenance is indeed costly, and creating maintainable code is an art. Adding a comment is too often an indicator that I'm failing at ensuring the code supports future comprehension. I now reflect each time I feel compelled to add a comment, to see if perhaps there's a better way. Sometimes, there is; sometimes, there is not.

To join in on a discussion on comments within code go to the CodeGuru discussion forum: General Discussion > General Developer Topics http://www.codeguru.com/forum/showthread.php?t=415553.

About the Author

Jeff Langr is a veteran software developer with a score and more years of experience. He's authored two books and dozens of published articles on software development, including Agile Java: Crafting Code With Test-Driven Development (Prentice Hall) in 2005. You can find out more about Jeff at his site, http://langrsoft.com, or you can contact him directly at jeff@langrsoft.com.
Share:
Home
Mobile Site | Full Site
Copyright 2017 © QuinStreet Inc. All Rights Reserved