Dr. Dobb's | Gotchas #1 and #17

Gotchas #1 and #17

Excerpted from C++ Gotchas: Avoiding Common Problems in Coding and Design. Steve Dewhurst looks at excessive commenting and maximal munch.

May 01, 2003
URL:http://www.drdobbs.com/gotchas-1-and-17/184403953

May 2003/C++ Gotchas #1 and #17

Gotcha #1: Excessive Commenting

Many comments are unnecessary. They generally make source code hard to read and maintain, and frequently lead maintainers astray. Consider the following simple statement:

a = b;  // assign b to a

The comment cannot communicate the meaning of the statement more clearly than the code itself, and so is useless. Actually, it's worse than useless. It's deadly. First, the comment distracts the reader from the code, increasing the volume of text the reader has to wade through in order to extract its meaning. Second, there is more source text to maintain, since comments must be maintained as the program text they describe is modified. Third, this necessary maintenance is often not performed.

c = b; // assign b to a

A careful maintainer cannot simply assume the comment is in error and is obliged to trace through the program to determine whether the comment is erroneous, officious (c is a reference to a), or subtle (assigning to c will later cause the same assignment to be propagated to a somehow). The line should originally have been written without a comment:

a = b;

The code is maximally clear as it stands, with no comment to be incorrectly maintained. This is similar in spirit to the well-worn observation that the most efficient code is code that doesn't exist. The same applies to comments: the best comment is one that didn't have to be written, because the code it would otherwise have described is self-documenting.

Other common examples of unnecessary comments frequently occur in class definitions, either as the result of an ill-conceived coding standard or as the work of a C++ novice:

class C {
 // Public Interface
 public:
   C(); // default constructor
   ~C(); // destructor
   // . . .
};

You get the feeling you're reading someone's crib notes. If a maintainer has to be reminded of the meaning of the public: label, you don't want that person maintaining your code. None of these comments does anything for an experienced C++ programmer except clutter the code and provide more source text to be improperly maintained.

class C {
 // Public Interface
 protected:
   C( int ); // default constructor
 public:
   virtual ~C(); // destructor
   // . . . 
};

Programmers also have a strong incentive not to "waste" lines of source text. Anecdotally, if a construct (function, public interface of a class, and so on) can be presented in a conventional and rational format on a single "page" of about 30-40 lines, it will be easy to understand. If it goes on to a second page, it will be about twice as hard to understand. If it goes on to a third page, it will be approximately four times as hard to understand.

A particularly odious practice is that of inserting change logs as comments at the head or tail of source code files:

/* 6/17/02 SCD fixed the gaforniflat bug */

Is this useful information, or is the maintainer just bragging? This comment is unlikely to be of any use whatever within a week or two of its insertion, but it will hang on grimly for years, distracting generations of maintainers. A much better alternative is to cede these commenting tasks to your version control software; a C++ source code file is no place to leave a laundry list.

One of the best ways to avoid comments and make code clear and maintainable is to follow a simple, well-defined naming convention and choose clear names that reflect the abstract meaning of the entity (function, class, variable, and so on) you're naming. Formal argument names in declarations are particularly important. Consider a function that takes three arguments of identical type:

/*
 Perform the action from the source to the destination.
 Arg1 is action code, arg2 is source, and arg3 is destination.
*/
void perform( int, int, int );

Not too terrible, but think what it would look like with seven or eight arguments instead of three. We can do better:

void perform( int actionCode, int source, int destination );

Better, though we should probably still have a one-liner that tells us what the function does (though not how it does it). One of the most attractive things about formal argument names in declarations is that they, unlike comments, are generally maintained along with the rest of the code, even though they have no effect on the code's meaning. I can't think of a single programmer who would switch the meanings of the second and third arguments of the perform function without also changing their names, but I can identify legions of programmers who would make the change without maintaining the comment.

Kathy Stark may have said it best in Programming in C++: "If meaningful and mnemonic names are used in a program, there is often only occasional need for additional comments. If meaningful names are not used, it is unlikely that any added comments will make the code easy to understand."

Another way to minimize comments is to employ standard or well-known components:

printf( "Hello, World!" ); // print "Hello, World" to the screen

This comment is both useless and only occasionally correct. It's not that standard components are necessarily self-documenting; it's that they're already well documented and well known.

swap( a, a+1 );
sort( a, a+max );
copy( a, a+max, ostream_iterator<T>(cout,"\n") );

Because swap, sort, and copy are standard components, additional comments inserted above can only clutter the source and introduce imprecision in the description of the standard operations.

Comments are not inherently harmful — and are often necessary — but they must be maintained, and they're typically harder to maintain than the code they document. Comments should not state the obvious or provide information better maintained elsewhere. The goal is not to eliminate comments at any cost but to employ the minimal volume of comments that permits the code to be readily understood and maintained.

Gotcha #17: Maximal Munch Problems

What do you do when faced with an expression like this?

++++p->*mp

Have you ever had occasion to deal with the "Sergeant operator"?

template <typename T>
class R {
  // . . . 
  friend ostream &operator <<< // a sergeant operator?
    T >( ostream &, const R & );
};

Have you ever wondered whether the following expression is legal?

a+++++b

Welcome to the world of maximal munch. In one of the early stages of C++ translation, the portion of the compiler that performs "lexical analysis" has the task of breaking up the input stream into individual "words," or tokens. When faced with a sequence of characters like ->*, the lexical analyzer might reasonably identify three tokens (-, >, and *), two tokens (-> and *), or a single token (->*). To avoid this ambiguous state of affairs, the lexical analyzer always identifies the longest token possible, consuming as many characters as it legally can: maximal munch.

The expression a+++++b is illegal, because it's tokenized as a ++ ++ + b, and it's illegal to post-increment an rvalue like a++. If you had wanted to post-increment a and add the result to a pre-incremented b, you'd have to introduce at least one space: a+++ ++b. If you have any regard for the readers of your code, you'll spring for another space, even though it's not strictly necessary: a++ + ++b, and no one would criticize the addition of a few parentheses: (a++) + (++b).

Maximal munch solves many more problems than it causes, but in two common situations, it's an annoyance. The first is in the instantiation of templates with arguments that are themselves instantiated templates. For example, using the standard library, one might want to declare a list of vectors of strings:

list<vector<string>> lovos; // error!

Unfortunately, the two adjacent closing angle brackets in the instantiation are interpreted as a shift operator, and we'll get a syntax error. Whitespace is required:

list< vector<string> > lovos;

Another situation involves using default argument initializers for pointer formal arguments:

void process( const char *= 0 ); // error!

This declaration is attempting to use the *= assignment operator in a formal argument declaration. Syntax error. This problem comes under the "wages of sin" category, in that it wouldn't have happened if the author of the code had given the formal argument a name. Not only is such a name some of the best documentation one can provide, its presence would have made the maximal munch problem impossible:

void process( const char *processId = 0 );

About the Author

Stephen C. Dewhurst (<www.semantics.org>) is the president of Semantics Consulting, Inc., located among the cranberry bogs of southeastern Massachusetts. He specializes in C++ consulting, and training in advanced C++ programming, STL, and design patterns. Steve is also one of the principal lecturers at The C++ Seminar ().