Treat Comments With Suspicion

When a comment is needed for clarity, the ExtremeWay is to improve the code until, ideally, it no longer needs a comment. Understand what it is ToNeedComments.

Do not comment bad code, rewrite it.

It's not always possible to rewrite unclear code. Examples:

• Some steps need to be executed in a nonobvious order, because the obvious order is significantly slower. (If you don't comment such code, often you'll be the first to shake your head about it and change it back to the slower way :/ )

• You must deal with an unusual interface, e.g. an iterating function starting (conceptually) at -1 or 2. See Microsoft's FindFirstFile?/FindNextFile? functions and the CFileFind class. Try creating an enum for FirstXXX rather than hard coding the value and then commenting it. There are lots of coding solutions that can be applied before falling back onto comments. Now you're going to need to put the same comment for the enum; the comment still says something you can't say in code (without being absurd). Try the enum approach and see if it is even possible to write a comment without duplicating what is written in the code.

• You are using a new, unfamiliar idiom/pattern. You yourself might recognize it, but what about the unfortunate fellow programmer seeing this way-to-do-it the first time in his or her life? Why should the reader of the class care where you got the pattern for the code? If the pattern is making the code complex, don't use it!

-- FreddyTheCat?

Explaining the programming language isn't one of the exceptions. It's OK to assume competence but not wizard status. On the other hand, using the most obscure form of the language isn't OK either. If the idiom is important and not uncommon, use it. Give the method a decent name, sort of like (someone correct my code, I'm just typing this in)

 copyToNull (char* In, char* Out) {
   while( *Out++ = *In++ );
 }

 exportEmployees
   employees do: [ :each | each exportOptimized ]

 Employee>>exportOptimized
   self exportPersonalBuffered.
   self exportDollarsBuffered.

 Employee>>exportPersonalBuffered
   | bufferedPersonData |
   bufferedPerson := self getPersonalData.
   bufferedPerson exportName.
   bufferedPerson exportAddress.
   bufferedPerson exportPhone.

 Employee>>exportDollarsBuffered
   etc ...

Bjarne Stroustrup says in "The C++ Programming Language" (3rd Ed., Chapter 6.4):

If something can be described in the language itself, one should do that, and not just mention it in a comment.

To pick up your code example, the "optimized" in exportOptimized is ambiguous. It can refer to the action done, or to the way the code is written. The latter usage smells, like HungarianNotation, and is exposing implementation details. If I see a name like exportOptimized, I assume the function exports something in an optimized form (e.g. a database without deleted records), I don't recognize it as a warning not to meddle with the code. See also Clear Warning in MethodCommenting.

I don't want to duplicate MethodCommenting and ToNeedComments here. Many caveats about TreatCommentsWithSuspicion that are in my mind now are expressed there, in the patterns at the beginning.

Just now I stumbled over a possible reason as to why I oppose TreatCommentsWithSuspicion and "Do not comment bad code, rewrite it.": Both statements are negative. How about "If you can turn a comment into source code, do it."? This guideline is based on the Stroustrup quote, but goes farther, as the quote doesn't say anything about removing the comment. With my proposed guideline, there is less danger of losing information when removing comments, a danger I see with the more urgent statements above. <sarcasm mode> But maybe the sense of urgency lies within me, because I am one of those rule-obeying Krauts :). </sarcasm mode>

A good partner for "If you can turn a comment into source code, do it." would be: "If you can make code clearer, do it. Even if you just add a comment.". This also goes with NanoRefactorization.

-- FreddyTheCat?

I like the positive form. The partner rule I cannot support. It's easier to add a comment than it is to fix the code, but fixing the code is the right thing to do whenever possible. I would choose not to recommend the easy but wrong fix. Prefer telling people they can't comment unless all else fails ... because it rarely does. -- RonJeffries

I usually use a multi-phase approach. Every time there's some code that's unclear, I comment it. Then, every time I run across a comment, I see if there's an ExtractMethod or renaming I could do to make the code say exactly the same thing as the comment. Then once I get code that looks like this

  // Load register state for current process
  currentProcess.loadRegisterState();

I tend to get better naming and more SelfDocumentingCode this way, because I'm writing first for the human and then turning the human code into something the machine can read. It's the same reasoning behind MakeItWorkMakeItRightMakeItFast. Focus on achieving the goal (clarity) first, and then refactor so there's no duplication.

-- JonathanTang

Almost every sort of collection in Java is 0-based. The notable exceptions are the JDBC API and Record stores. Every time I have cause to iterate or select by index in one of its one-based collections, I comment it. Mostly to remind myself that these are the exceptional cases and I can't be sloppy. -- StevenNewton (and KarlKnechtel)

Are comments SignOfInsecurityInCode?

This page mirrored in WikiPagesAboutRefactoring as of July 17, 2004