Big Blocks Of Asterisks

A terrible commenting style, lambasted in CodeComplete and MassiveFunctionHeaders. 

  /***********************************************************************
   * FUNCTION: gettime                                                   *
   *                                                                     *
   * PURPOSE: Return the number of seconds elapsed since the start of    *
   * 1970.                                                               *
   *                                                                     *
   * RETURN: Current unix time.                                          *
   *                                                                     *
   * PARAMETERS: None.                                                   *
   *                                                                     *
   * COPYRIGHT: 1901 (C) Yoyodyne Corp.                                  *
   *                                                                     *
   * REVISION: 1.1, 1 April 1901                                         *
   *                                                                     *
   * AUTHOR: C. Rative Hacket                                            *
   *                                                                     *
   * REVISION HISTORY:                                                   *
   *     1.0 - 1 April 1900 by C. Rative Hacket                          *
   *           Written and documented, in standard form.                 *
   *     1.1 - 1 April 1901 by J. Random Hacker                          *
   *           Changed spacing to conform to new corporate coding        *
   *           standard.                                                 *
   *                                                                     *
   * PSEUDO-CODE:                                                        *
   *     1. Obtain current system time.                                  *
   *     2. Return it.                                                   *
   *                                                                     *
   * SEE ALSO:  "man sys_gettime" for more information.                  *
   ***********************************************************************/
  int gettime() {
    return sys_gettime();
  }

The one advantage of this style is that you can tell what's in a comment when grepping through your source files. Of course, any decent editor (i.e. emacs) will do the asterisks for you automatically.

Personally, I think that function / class / method header block comments are a good thing, and ought to be used in every instance. But the real failure of BadCodingStandards is to say that all the entries are mandatory. There's no need to document the obvious or trivial, which is why the following comment is worse than useless: 

  ++i;  // increment the variable "i".
I pretty much consider everything except name and description (function/purpose above) optional, and wouldn't bother with revision history unless the function required very careful maintenance. (...in which case one should consider rewriting it to make it simpler.)

(I agree that comment is terrible. It should really be: 

  ++i;  // pre-increment the variable "i"
)

Another big problem with the example shown above is the asterisks along the right side of the box: Every change you make to any comment in the block requires that you realign the right hand asterisks so they all line up again -- a complete waste of time. -- JeffGrigg

The name is worse than useless. It's redundant. The function name is immediately listed after the comment. All you need is a description, which might include parameter usages. -- SunirShah

Jeff, please apply your standard to creation of a suitable block for the example function on this page. Then discuss what is added by that block. Thanks!

-- SomeExtremist?, I guess. ;->

I would probably document this function like this: 

  ///////////////////////////////////////////////////////////////////////////////
  //  Function:   gettime
  //  Description:
  //      Return the number of seconds since midnight, the morning of 1/1/1970,
  //      universal time.  See "time_t" system data type for details.
  //
  time_t gettime()
  {
      return sys_gettime();
  }
On the value of these items: This particular function needs no further documentation.

Actually, I have to wonder why this function was written at all: it's quite useless; just call sys_gettime() yourself.

Making sure all calls go through this one function means that if you want to test the system by running time fast, or running time slow, then you can replace this one call with a fake version. Then you can control how fast or slowly time seems to pass. Otherwise you would have to find and replace every call to sys_gettime() in your system. Making sure all system calls go through your own wrapper seems essential when writing code that needs to be portable and future proof.

I was recently forced by a big corporate QA department to document every parameter of every method. Most of it was useless: well-named parameters are largely self-documenting. But sometimes parameters can use some additional explanation: Is a NULL value valid for this pointer? Is the index zero or one based? What are the valid values for a control code (...if not made obvious by an enumerated type)? Even if an enumerated type is used, are only some of the possible values valid?

If a function/method gets too complex or has a tricky implementation, I add a pseudo-code summary. (Sometimes refactoring is a better solution, but not always.)

Some documentation of return values, especially exceptional values (like NULL for "string not found") and types of exceptions thrown can be useful. (I haven't found the C++ "throws()" clause very helpful: Most mid-level functions may throw exceptions meaningful to that level "and anything that any of the low level service routines might throw." So all the "throws()" clauses end up being generic, and not very informative.)

-- JeffGrigg

I would recommend against reusing a variable as a status or error value. Returning a NULL pointer is one example. Another is to return a negative value for a normally positive value. Both of these lead to non-obvious problems when the caller does not validate the return value. Alternatives: use exceptions or provide a separate error or status return. I prefer a boolean, but a status code could also be used. You can trade off whether to have the status as a return value or as a function parameter and I find I switch between the two depending upon what I am doing. Separating the data from the status provides a much stronger hint to the caller that he should check the status than a comment provides. -- WayneMack

If you can find a single piece of useful information that is in any of the above that is not in the following you will be eligible to win a prize: 

    typedef time_t SecondsSinceJanuaryFirst1970;


    SecondsSinceJanuaryFirst1970 gettime() {
        return sys_gettime();
    }
Thinking of comments as smells is a great way to discover poorly named symbols, such as "time_t". (Hence the typedef).

It doesn't say what time on January 1st 1970 it's counting from - what is my prize? -- JamesKeogh

The rule of not stating the obvious would imply that the default time of a day is its beginning. See also NearestFittingContext.

Repeating that the function takes no parameters and returns an int is obvious and trivial. Version and author history should be in your version-control system. If the name doesn't adequately convey the description and you honestly can't refactor or make up a better name, then it's good to add a description. If you manage to eliminate all the other elements then there's no point repeating the function name. -- MartinPool

That's why I kinda like the JavaDoc approach where you don't just say 

@param int id
You say 

@param id - the identifier of the widget we are requesting.
So the comments give you semantics of the parameters, not syntax. Any decent IDE has pop-up text to tell you the syntax of the parameters, what I want is some semantics :-) -- AlanFrancis

Compare to naming the variable requestedWidgetId, please.

Well.... Where I work we get a lot of "lower-level" code from our parent company in Japan, so while the example above is trivial, some of the code (which I don't have the luxury of changing) is not.

The best I can do is to take the header file we're provided (along with the Video Encoder DLL or the File System driver dll) and fill it with JavaDoc style comments doing what Jeff mentioned above. Documenting what a parameter is for, if it's a pointer is it an in or out param. Who allocates the memory and who frees it? Is NULL valid? What conditions will return an error etc.

As I say, I'd write my code better (wouldn't we all :-) , but when I have to document someone else's code, I use the style I discuss above... document semantics, not syntax. -- AlanFrancis

If a function/method gets too complex or has a tricky implementation, I add a pseudo-code summary. (Sometimes refactoring is a better solution, but not always.)

When is refactoring not as good a solution as pseudo-code? I assume if you can change the comments, you can refactor the code (at least inside that module or object). The only time I personally would write pseudo-code instead of refactoring is where I know (from profiling) that the tricky implementation is needed for acceptable performance.

My biggest complaint about this style of documentation is that it is discourages refactoring by adding to the amount of work required to add functions, and thereby encourages the MonsterSubroutine. Once a body of code is in this style, it is a back-breaking task to change it. A similar problem is a difficult make system, where adding a file requires editing many make files, and even copying the file to multiple locations. This encourages the MonsterFile? syndrome. -- RobertField

Check out the discussion on SelfDocumentingCode and other pages.

You guys keep talking about putting comments into the code describing the limits for arguments, the return values under certain conditions, etc. Uh, isn't this what API documents are for? When I look up a commercial library class method I want to see what all this good stuff is without having to dig into the source file itself. Eh? Why should it be any different for the code I maintain or create from scratch? The XP folk will probably jump on that one.

Because very few projects have the luxury of creating API documents for all the interfaces used internally to a product. If you're working in C or C++, assertions are often a good way of documenting restrictions on input parameters .

More importantly, because when "API documents" and the source code don't agree, the source code is always right - if you do what the API documents tell you to do, your program will crash; if you do what the code tells you to do, the API documents won't.

Whoa, ho, ho! Up above we were talking about the use of somebody else's code that we don't have the luxury of changing. (This is not really an XP shop around here.) If I had access to some API of some kind that explicitly told me how a layer in the comm system was supposed to work I could have saved myself literally days - maybe weeks - of work. I had to dig around and muddle through the code because all of this "simple" real-time, event-driven stuff was so durn complicated that nobody other than the author knew how it worked. And even he had to go looking back through the code again to find out.

Now: if I had a nice API to look up it would save me a lot of headaches. Once a functional mechanism gets to be working you should be able to count on its presentation to remain the same, no? If you are going to be changing the interface then you also change the functionality. Uhh, we are getting a bit afield from the BigBlocksOfAsterisks. -- MS

What I'd really like to see is that the comments/syntax blend more in languages. For example, in java I want to write a function like:

/** 

 *  Gets a foo from the store
 *  @param Store store - The store to retrieve the foo from
 *  @return Foo - An instance of foo
 */
private foo { 
  ...
}

The exact syntax needs work, but this way it is easier to keep your comments and parameters in sync.

-- DaveTauzell

Why that, when the following code documents itself without comments?: 

  private Foo getFooFromStore(Store store) {
    ...
  }
Or, even better 

  Store { ...
      public Foo getFoo() {         
      }
  }

Extremely Short: 

  foo()
It is the other extreme: it is undocumented code, but it leaves almost everything unsaid and undocumented. It will only be useful when you RefactorMercilessly. This will occur only if some plan exists which spells out what is wanted, when and where it is to be produced, and the authority and resources to do it.

Whenever documentation is included in code, it is usually for future reference, legal requirments (ownership, distribution, etc.), or for current tracking purposes when used to mark and aid in completing things ToDo. It is written by the coder (presumably a knowledgeable programmer) for use by a future (presumably able programmer) to explain code and to provide, as it were, abstract one-liners, information which will "help" those who will maintain or modify the code by providing clarifications of methods, structure and dependencies existing in the code segment. It will fail to do this if it is incomplete, out-of-date, unclear, or worse, completely missing.

-- DonaldNoyes 20080114

Now here's a fun one. Prize for the first one who understands what this does:

barrier()

the same as foo()

Related:

BenefitsOfHeaderComments CommentTheWhy BigBlocksOfAsterisks MassiveFunctionHeaders SmallFunctionHeaders MethodCommenting MeaningfulComment FileHeaders SelfDocumentingCode

External References:

CategoryDocumentation

EditText of this page (last edited January 19, 2008) or FindPage with title or text search