Larry Osterman's WebLog

Confessions of an Old Fogey
Blog - Title

What does style look like, part 5

What does style look like, part 5

Rate This
  • Comments 14
Continuing the discussion of "style"..

The other day, I talked a bit about how comments affect the "style" of a piece of code.

Today, I want to talk  about the headers that accompany each routine and file.

And yes, every routine and file needs to have a block comment header around it.  If you don't , then the next person who gets to maintain the file curses you.

At a minimum, the file header should have a brief summary of what the code in the file does, and probably a copyright notice.

For subroutines, the name of the routine and a brief (or maybe not-so-brief) description of what it does.

Beyond that, anything goes.  I've seen literally dozens of variants of styles.  Some of my oldest code has the following for routine headers (written in Bliss36, so ! is a comment character):

! Copyright (C) 1982, Lawrence Osterman
! CHANGE LOG
!-------------------------------------------------------------------
! 9 Nov 82 OSTERMAN Added a check to see if you are watching
!                   a user when you auto add them.

Each routine had:
!++
! Get_Accounts_JFN :
!
! Input:
!    None
! Output:
!    None
! Side effects:
!    None
!
!--
 

Not the cleanest, but... 

Unfortunately, I don't have any good examples of a module header, typically one looks like:

/*++
 *        <Copyright Notice>
 *
 *  Module Name:
 *        <Name of module filename>
 *
 *  Abstract:
 *        <Brief abstract for module>
 *
 *  Author:
 *        <Authors name and email>
 *
 *  Environment:
 *        <intended environment for this module (optional)>
 *
 *  Notes:
 *        <optional notes>
 *
 *  Revision History:
 *
 *    <Author> <Change Date> <Change Description>
 --*/
 

The "Author" field can be controversial - Since modules can change owners, it's not clear if there's long-term value for the owner field.  Similarly, the revision history is controversial.  It can be extraordinarily useful, but only if maintained diligently.

My earliest copy of Microsoft documentation (from 1984) has the following example of a subroutine header:

/***    name - very brief description
 *
 *     <description>
 *
 *     name    (parm1, parm2, parm3, ..., parmn)
 *
 *     ENTRY   parm1 - description
 *             parm2 - description
 *     EXIT    parm3 - description
 *             .
 *             parmn - description
 *
 *     <discussion of internals, one or more paragraphs>
 *
 *     WARNING:    <discussion of limitations, gotchas, etc.>
 *
 *     EFFECTS:    <'none', or discussion of global effects>
 *
 */

Again, this was 1984, but in general, the concepts involved are the same.  This block has all of the requirements for the routine header included.  Beyond that, it's all esthetics.

Here are some styles I've seen (building on the early coding standard above).

/*++    name - very brief description
 *
 *     <description>
 *
 *     name    (parm1, parm2, parm3, ..., parmn)
 *
 *     ENTRY   parm1 - description
 *             parm2 - description
 *     EXIT    parm3 - description
 *             .
 *             parmn - description
 *
 *     <discussion of internals, one or more paragraphs>
 *
 *     WARNING:    <discussion of limitations, gotchas, etc.>
 *
 *     EFFECTS:    <'none', or discussion of global effects>
 *
 --*/

As I said, this is all about esthetics, so everyone has their own favorites.

/********************************************************************\
*                                                                    *
*    name - very brief description                                   *
*                                                                    *
*     <description>                                                  *
*                                                                    *
*     name    (parm1, parm2, parm3, ..., parmn)                      *
*                                                                    *
*     ENTRY   parm1 - description                                    *
*             parm2 - description                                    *
*     EXIT    parm3 - description                                    *
*             .                                                      *
*             parmn - description                                    *
*                                                                    *
*     <discussion of internals, one or more paragraphs>              *
*                                                                    *
*     WARNING:    <discussion of limitations, gotchas, etc.>         *
*                                                                    *
*     EFFECTS:    <'none', or discussion of global effects>          *
*                                                                    *
\********************************************************************/

This one's hard to maintain, because of the trailing *'s, but it is pretty.  It might do for a file header.

And of course, there's the version with just C++ comments:

//-----------------
//    name - very brief description
//
//     <description>
//
//     name    (parm1, parm2, parm3, ..., parmn)
//
//     ENTRY   parm1 - description
//             parm2 - description
//     EXIT    parm3 - description
//             .
//             parmn - description
//
//     <discussion of internals, one or more paragraphs>
//
//     WARNING:    <discussion of limitations, gotchas, etc.>
//
//     EFFECTS:    <'none', or discussion of global effects>
//
//---

In general, if I had to come up with rules for what makes subroutine headers work, I'd say that they had to be descriptive, they had to include documentation for all the parameters, they had to have some consistent style to differentiate them from normal code.

One other thing that effects subroutine headers is the existance of tools like AutoDOC.  If you choose to use an auto-documentation tool like autodoc (and there are dozens of them), then your subroutine headers can form the basis of the internal documentation for your module.  It doesn't change the fact that you've got to have design specifications, but it does provide an external reference tool.

So lets go back to the commented code and add some header and routine comments:

/*++
 *        <Copyright Notice>
 *
 *  Module Name:
 *        MyProgram.c
 *
 *  Abstract:
 *        This program implements a solution to the Josephus problem. 
 *        Code taken from Robert Sedgewick's "Algorithms in C, Third Edition",
 *        Program 3.13, on Page 103.
 *
 *  Author:
 *        Robert Sedgewick
 *
 *  Revision History:
 *
 *    LarryO             11/12/2004     Restructured for weblog posts.
 *    Robert Sedgewick   <Unknown>      Created. 
 --*/

#include "list.h"


/*++    main - Implements the Josephus problem and prints the result.
 *
 *
 *     main (cArg, rgszArg)
 *
 *     ENTRY   cArg - number of arguments.
 *             rgszArg - array of cArg null terminated strings
 *     EXIT    None.
 *
 *     <discussion of internals, one or more paragraphs>
 *
 *
 --*/
main(C cArg, SZ rgszArg[])
{
    I iNode;
    I cNodes = atoi(rgszArg[1]);
    I cNodesToSkip = atoi(rgszArg[2]);
    PNODE pnodeT;
    PNODE pnodeCur;

    InitNodes(cNodes);

    //
    // Create a list of cNodes nodes. 
    // 
    for (iNode = 2, pnodeCur = PnodeNew(1); iNode <= cNodes ; iNode++)
    {
        pnodeT = PnodeNew(iNode);
        InsertNext(pnodeCur, pnodeT);
        pnodeCur = pnodeT;
    }

    //
    // Walk the list of nodes, freeing the node that occurs at every cNodesToSkip nodes in the list.
    //
    while (pnodeCur != PnodeNext(x))
    {
        for (iNode = 1; iNode < cNodesToSkip ; iNode++)
        {
            pnodeCur = PnodeNext(pnodeCur);
        }
        FreeNode(PnodeDeleteNext(pnodeCur));
    }

    //
    // Print out the value of the current node.
    //
    printf("%d\n", Item(nodeCur));
}

That's quite different.  Tomorrow, we start looking at naming conventions.

 

  • UGH, version history and authorship are some of the most worthless items to encode in the file itself.

    The file is stored (or had better be) in a revision control system. Any system worth anything will be able to give you the history of a given file. And I'm sure that what y'all use at MS can give you a line-by-line breakdown of when a particular line was committed, and by whom. CVS and ClearCase both can.

    Besides, the revision history and authorship is generally pretty useless outside of the development team. So why expose it (through header files)?

    Plus, the embedded history soon gets to be as large as the file itself - making it difficult to read.
  • The thing is that sometimes there are people who get the file without access to the source code control system, so they CAN be valuable. Just not always.
  • I disagree with BFW. I prefer to have information localized to were the changes are made.

    Over the years I have experimented with many function header styles and have finally settled on this one:

    ///////////////////////////////////////////
    // function name
    //
    // description: optional, not every function needs a description
    //
    // revision rr 0411.15 my initials, date modified
    // reason for modification
    ///////////////////////////////////////////

    I prefer to see the revision history at the function since it allows me to quickly see the history of the function (rather than tracking it at the top of the file, or through version control software. It also lets me know if this is a problem prone function or not.

    Having the developer initials also lets me know who is responsible for something (of course, I've worked on teams whose members can easily be identified by their initials - might be tough at MS though). Sure, eventually, the information is meaningless.

    If the function level revision history becomes too busy, I simply delete the older entries.

    I've given up trying to document the parameters and return values, since they quickly get out of step with the function.

    The description should describe in a sentence or two what is going on. Detailed comments should be part of the body.

    No doubt everyone has their preference or bias (some have no style whatsoever).

    I also use the modified BSD formatting style (wow, I didn't even know it had a name, I just thought it was my own bizarre style developped over the years).

    rr
  • You can let the SCM autogenerate the version-info etc. In Visual SourceSafe it would be something like this (see 'Expand keywords' in the docs):

    /*++
    * <Copyright Notice>
    *
    * Module Name:
    * $Workfile: $
    *
    * Abstract:
    * This program implements a solution to the Josephus problem.
    * Code taken from Robert Sedgewick's "Algorithms in C, Third Edition",
    * Program 3.13, on Page 103.
    *
    * Author:
    * Robert Sedgewick
    *
    * Revision History:
    *
    * $History: $
    --*/

    Using CVS it's quite similar. There are a lot more keywords available as well.

    I prefer to put a $Header: $ at the top of the module, which expands into this: "$Header: Logfile, Revision, Date, Author$". Makes it very easy to see the revision nr. of the module + who last changed it.
  • Andreas, that's actually REALLY cool, I didn't realize that. I wonder if something like that works in our internal source code control system - I should look to find out.

  • Alas, function-level or file-level comments are only useful if your cow-orkers give a @#%!* about documenting their source code... *grumble*
  • What, you don't use VSS for Windows?!?! :P (*)

    Maybe I'm wrong here, but I've read that your interanl SCM is based on Perforce. Perforce supports keyword expansions as well. See http://www.perforce.com/perforce/technotes/note054.html :)
  • I use Doxygen comments for files and functions/classes etc. They are perfectly readable and at the same time produce very nice crossreferenced documentation in HTML, CHM, PDF or LaTeX format. For a method a comment can look like this:

    /**
    * Description of the method, cross-references are automatically hyperlinked.
    *
    * @param[in] arg1 Description
    * @param[out] arg2 Description
    *
    * @return Description of return value
    * @retval value1 Description
    * @retval value2 Description
    *
    * @throw exception1 Description
    */

    There are lots of other supported tags, and Doxygen can parse C/C++, Java etc.

    Doxygen is free: http://www.doxygen.org
  • I had a revelation last night about another reason it's a _really bad idea_ to make changes to code just to reindent/name it: Anyone who keeps diff patches against codebases will be forced to rematch and recode them completely. I've seen a lot of patches or completely expanded programs out there that keep the revision maintenence pain down with diff patches.

    I'm always trying different function comments, I've settled on this basic format for mine (see html source for the spacing):

    //* fx_html_ansi2entities()
    /// Converts ascii to HTML. (see fx_html_entities2ansi)

    /// Arguments: (All by-value)
    /// $text: [string] Text to convert, string or array.
    /// $dohtml: [bool] (Optional) Convert HTML chars: & < > (bad if used on existing html) (def = false)
    /// $doent: [bool] (Optional) Convert invalid chars to entities (no multi-byte support) (def = true)
    /// $ignoretags: [bool] (Optional) WHen $dohtml is true, only escapes non-HTML, ignoring real HTML. (def = false)
    /// $extent: [bool] (Optional) Convert invalid chars to entities (no multi-byte support) (def = false)

    /// Returns:
    /// [string] Text with encoded entities.

    And built a mini-javadoc tool that turns that into various formats.

    For some reason I have an absolute antipathy to blank comment lines. I just break it instead.
  • Andreas, at one time, I believe our SCS shared a common source with perforce, but... I'm absolutely going to investigate.


    We're currently using DocJet on our project, other groups use different tools. The cool thing about docjet is that it doesn't require special formatting characters.

  • File and function headers are useful documentation, but I'll agree with an earlier post that the author and revision info should not be inside the file. A version control system (SourceSafe,CVS,Perforce,whatever) can maintain the revision history for you, and *keep it up to date*. In addition, if you want to ensure that the history is available outside the version control system, then use a ChangeLiog file in addition to the version control history, and ensure that your tools update the ChangeLog text file with the same info as the revision history.

    As an example of why keeping the revision hostory ina file is a bad idea, I have seen it actually cause a bug in code. The expansion of the history by CVS extended the header comment by enough lines to change the assembly level representation of __LINE__ in a debug build, causing the code layout to be incompatible with other parts of the (embedded) system, and the code to crash...
  • As a Doxygen user, I don't put the function name in my function comments. It seems a bit redundant to me since it's written right below in the function header anyway.
  • Yesterday , I started talking about software contracts. Today I'd like to start talking about how contracts

Page 1 of 1 (14 items)