Skip to Content
Software EngineeringBooksClean CodeCh 4. Comments

Ch 4. Comments

Comments Do Not Make Up for Bad Code

Don’t comment bad code—rewrite it.

  • 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.

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. It is possible to make the point that programmers should be disciplined enough to keep the comments in a high state of repair, relevance, and accuracy.
  • Inaccurate comments are far worse than no comments at all. They delude and mislead. They set expectations that will never be fulfilled. They lay down old rules that need not, or should not, be followed any longer.
    • Truth can only be found in one place: the code. Only the code can truly tell you what it does.

Explain Yourself in Code (not in comment)

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

// v.s.

if (employee.isEligibleForFullBenefits())
  • It takes only a few seconds of thought to explain most of your intent in code.
  • In many cases it’s simply a matter of creating a function that says the same thing as the comment you want to write.

Good Comments

  • Keep in mind, however, that the only truly good comment is the comment you found a way not to write.
  • Sometimes our corporate coding standards force us to write certain comments for legal reasons.
    • For example, copyright and authorship statements are necessary and reasonable things to put into a comment at the start of each source file.
    • Where possible, refer to a standard license or other external document rather than putting all the terms and conditions into the comment.

Informative Comments

  • It is sometimes useful to provide basic information with a comment.
  • For example, consider this comment that explains the return value of an abstract method:
// Returns an instance of the Responder being tested.
protected abstract Responder responderInstance();
  • A comment like this can sometimes be useful, but it is better to use the name of the function to convey the information where possible. The comment could be made redundant by renaming the function: responderBeingTested.
// format matched kk:mm:ss EEE, MMM dd, yyyy
Pattern timeMatcher = Pattern.compile("\\d*:\\d*:\\d* \\w*, \\w* \\d*, \\d*");
  • In this case the comment lets us know that the regular expression is intended to match a time and date that were formatted with the SimpleDateFormat.format function using the specified format string.
    • Still, it might have been better, and clearer, if this code had been moved to a special class that converted the formats of dates and times. Then the comment would likely have been superfluous.

Explanation of Intent

  • Sometimes a comment goes beyond just useful information about the implementation and provides the intent behind a decision. You might not agree with the programmer’s solution to the problem, but at least you know what he was trying to do.
// This is our best attempt to get a race condition
// by creating large number of threads.
 
for (int i = 0; i < 25000; i++) {
	WidgetBuilderThread widgetBuilderThread
      	= new WidgetBuilderThread(widgetBuilder, text, parent, faillag);
  	Thread thread = new Thread(widgetBuilderThread); thread.start();
}
...

Clarification

  • Sometimes it is just helpful to translate the meaning of some obscure argument or return value into something that’s readable.
  • In general it is better to find a way to make that argument or return value clear in its own right; but when its part of the standard library, or in code that you cannot alter, then a helpful clarifying comment can be useful.
assertTrue(a.compareTo(a) == 0); // a == a
assertTrue(a.compareTo(b) != 0); // a != b
assertTrue(ab.compareTo(ab) == 0); // ab == ab
assertTrue(a.compareTo(b) == -1); // a < b
assertTrue(aa.compareTo(ab) == -1); // aa < ab
assertTrue(ba.compareTo(bb) == -1); // ba < bb
assertTrue(b.compareTo(a) == 1); // b > a
assertTrue(ab.compareTo(aa) == 1); // ab > aa
assertTrue(bb.compareTo(ba) == 1); // bb > ba
  • There is a substantial risk, of course, that a clarifying comment is incorrect. So before writing comments like this, take care that there is no better way, and then take even more care that they are accurate.

Warning of Consequences

  • Sometimes it is useful to warn other programmers about certain consequences.
public static SimpleDateFormat makeStandardHttpDateFormat() {
  //SimpleDateFormat is not thread safe,
  //so we need to create each instance independently.
  SimpleDateFormat df = new SimpleDateFormat(
    "EEE, dd MMM yyyy HH:mm:ss z");
  df.setTimeZone(TimeZone.getTimeZone("GMT"));
  return df;
}
  • You might complain that there are better ways to solve this problem. I might agree with you. But the comment, as given here, is perfectly reasonable. It will prevent some overly eager programmer from using a static initializer in the name of efficiency.

TODO Comments

//TODO-MdM these are not needed
// We expect this to go away when we do the checkout model
protected VersionInfo makeVersion() throws Exception { return null; }
  • TODOs are jobs that the programmer thinks should be done, but for some reason can’t do at the moment.
  • It might be a reminder to delete a deprecated feature or a plea for someone else to look at a problem. It might be a request for someone else to think of a better name or a reminder to make a change that is dependent on a planned event.
  • Whatever else a TODO might be, it is not an excuse to leave bad code in the system.

Amplification

  • A comment may be used to amplify the importance of something that may otherwise seem inconsequential.
String listItemContent = match.group(3).trim();
// the trim is real important. It removes the starting
// spaces that could cause the item to be recognized
// as another list.
new ListItemWidget(this, listItemContent, this.level + 1);
return buildList(text.substring(match.end()));

Javadocs in Public APIs

  • There is nothing quite so helpful and satisfying as a well-described public API. If you are writing a public API, then you should certainly write good javadocs for it.
  • But keep in mind the rest of the advice in this chapter. Javadocs can be just as misleading, nonlocal, and dishonest as any other kind of comment.

Bad Comments

  • Most comments fall into this category. Usually they are crutches or excuses for poor code or justifications for insufficient decisions, amounting to little more than the programmer talking to himself.

Mumbling

  • Plopping in a comment just because you feel you should or because the process requires it, is a hack. If you decide to write a comment, then spend the time necessary to make sure it is the best comment you can write.
try {
  	String propertiesPath = propertiesLocation + "/" + PROPERTIES_FILE;
  	FileInputStream propertiesStream = new FileInputStream(propertiesPath);
  	loadedProperties.load(propertiesStream);
} catch(IOException e) {
  	// No properties files means all defaults are loaded
}
  • Our only recourse is to examine the code in other parts of the system to find out what’s going on.
  • Any comment that forces you to look in another module for the meaning of that comment has failed to communicate to you and is not worth the bits it consumes.
    • Apparently, if we get an IOException, it means that there was no properties file; and in that case all the defaults are loaded.
    • But who loads all the defaults?
    • Were they loaded before the call to loadProperties.load?
    • Or did loadProperties.load catch the exception, load the defaults, and then pass the exception on for us to ignore?
    • Or did loadProperties.load load all the defaults before attempting to load the file?
    • Was the author trying to comfort himself about the fact that he was leaving the catch block empty?
    • Or—and this is the scary possibilitywas the author trying to tell himself to come back here later and write the code that would load the defaults?
    • …

Redundant Comments

  • The comment probably takes longer to read than the code itself. It’s certainly not more informative than the code. It does not justify the code, or provide intent or rationale
// Utility method that returns when this.closed is true.
// Throws an exception if the timeout is reached.
public synchronized void waitForClose(final long timeoutMillis)
throws Exception {
	...
}

Misleading Comments

  • Sometimes, with all the best intentions, a programmer makes a statement in his comments that isn’t precise enough to be accurate. Consider for another moment the badly redundant but also subtly misleading comment we saw.
  • Did you discover how the comment was misleading? The method does not return when this.closed becomes true. It returns if this.closed is true; otherwise, it waits for a blind time-out and then throws an exception if this.closed is still not true.
  • This subtle bit of misinformation, couched in a comment that is harder to read than the body of the code, could cause another programmer to blithely call this function in the expectation that it will return as soon as this.closed becomes true. That poor programmer would then find himself in a debugging session trying to figure out why his code executed so slowly.

Mandated Comments

  • It is just plain silly to have a rule that says that every function must have a javadoc, or every variable must have a comment.
  • Comments like this just clutter up the code, propagate lies, and lend to general confusion and disorganization.
  • For example, required javadocs for every function lead to abominations:
/**
*
* @param title The title of the CD
* @param author The author of the CD
* @param tracks The number of tracks on the CD
* @param durationInMinutes The duration of the CD in minutes
*/
public void addCD(
  String title, String author, int tracks, int durationInMinutes) {
	CD cd = new CD();
	cd.title = title;
    cd.author = author;
    cd.tracks = tracks;
    cd.duration = duration;
    cdList.add(cd);
}

Journal Comments

  • Sometimes people add a comment to the start of a module every time they edit it. These comments accumulate as a kind of journal, or log, of every change that has ever been made.
  • Long ago there was a good reason to create and maintain these log entries at the start of every module. We didn’t have source code control systems that did it for us. Nowadays, however, these long journals are just more clutter to obfuscate the module. They should be completely removed.

Noise Comments

  • Sometimes you see comments that are nothing but noise. They restate the obvious and provide no new information.
/** The day of the month. */
private int dayOfMonth;
 
/** * Returns the day of the month.
*
* @return the day of the month.
*/
public int getDayOfMonth() { return dayOfMonth; }
  • As we read through code, our eyes simply skip over them. Eventually the comments begin to lie as the code around them changes.

Scary Noise

  • What purpose do the following Javadocs (from a well-known open-source library) serve? Answer: nothing. They are just redundant noisy comments written out of some misplaced desire to provide documentation.
/** The name. */
private String name;
 
/** The version. */
private String version;
 
/** The licenceName. */
private String licenceName;
 
/** The version. */
private String info;
  • Read these comments again more carefully. Do you see the cut-paste error? If authors aren’t paying attention when comments are written (or pasted), why should readers be expected to profit from them?

Don’t Use a Comment When You Can Use a Function or a Variable

// does the module from the global list <mod> depend on the
// subsystem we are part of?
if (smodule.getDependSubsystems().contains(subSysMod.getSubSystem()))
  • This could be rephrased without the comment as
ArrayList moduleDependees = smodule.getDependSubsystems();
String ourSubSystem = subSysMod.getSubSystem();
if (moduleDependees.contains(ourSubSystem))
  • The author of the original code may have written the comment first (unlikely) and then written the code to fulfill the comment. However, the author should then have refactored the code, as I did, so that the comment could be removed.

Position Markers

  • Sometimes programmers like to mark a particular position in a source file. There are rare times when it makes sense to gather certain functions together beneath a banner like this. But in general they are clutter that should be eliminated
  • Think of it this way. A banner is startling and obvious if you don’t see banners very often. So use them very sparingly, and only when the benefit is significant. If you overuse banners, they’ll fall into the background noise and be ignored.

Closing Brace Comments

  • Sometimes programmers will put special comments on closing braces.
  • Although this might make sense for long functions with deeply nested structures, it serves only to clutter the kind of small and encapsulated functions that we prefer.
  • So if you find yourself wanting to mark your closing braces, try to shorten your functions instead.

Attributions and Bylines

/* Added by Rick */
  • Source code control systems are very good at remembering who added what, when. There is no need to pollute the code with little bylines.
  • You might think that such comments would be useful in order to help others know who to talk to about the code. But the reality is that they tend to stay around for years and years, getting less and less accurate and relevant.
  • Again, the source code control system is a better place for this kind of information.

Commented-Out Code

  • Others who see that commented-out code won’t have the courage to delete it.
  • They’ll think it is there for a reason and is too important to delete. So commented-out code gathers like dregs at the bottom of a bad bottle of wine.
  • There was a time, back in the sixties, when commenting-out code might have been useful. But we’ve had good source code control systems for a very long time now. Those systems will remember the code for us. We don’t have to comment it out any more. Just delete the code. We won’t lose it. Promise.

HTML Comments

  • If comments are going to be extracted by some tool (like Javadoc) to appear in a Web page, then it should be the responsibility of that tool, and not the programmer, to adorn the comments with appropriate HTML.

Nonlocal Information

  • If you must write a comment, then make sure it describes the code it appears near.
  • Don’t offer system-wide information in the context of a local comment. Consider, for example, the javadoc comment below. Aside from the fact that it is horribly redundant, it also offers information about the default port. And yet the function has absolutely no control over what that default is.
  • The comment is not describing the function, but some other, far distant part of the system.
  • Of course there is no guarantee that this comment will be changed when the code containing the default is changed.
/**
 * Port on which fitnesse would run. Defaults to <b>8082</b>.
 *
 * @param fitnessePort
*/
public void setFitnessePort(int fitnessePort) {
  this.fitnessePort = fitnessePort;
}

Too Much Information

  • Don’t put interesting historical discussions or irrelevant descriptions of details into your comments.

Inobvious Connection

  • The connection between a comment and the code it describes should be obvious.
  • If you are going to the trouble to write a comment, then at least you’d like the reader to be able to look at the comment and the code and understand what the comment is talking about.
/*
* start with an array that is big enough to hold all
* the pixels (plus filter bytes), and an extra 200 bytes
* for header info */
this.pngBytes = new byte[((this.width + 1) * this.height * 3) + 200];
  • What is a filter byte? Does it relate to the +1? Or to the *3? Both? Is a pixel a byte? Why 200? The purpose of a comment is to explain code that does not explain itself. It is a pity when a comment needs its own explanation.

Function Headers

  • Short functions don’t need much description. A well-chosen name for a small function that does one thing is usually better than a comment header.

Javadocs in Nonpublic Code

  • As useful as javadocs are for public APIs, they are anathema to code that is not intended for public consumption.
  • Generating javadoc pages for the classes and functions inside a system is not generally useful, and the extra formality of the javadoc comments amounts to little more than cruft and distraction.
Last updated on
Clean codeCh 7. Error handling