Larry Osterman's WebLog

Confessions of an Old Fogey
Blog - Title

What does style look like, part 4

What does style look like, part 4

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

Yesterday, I showed the code reformatted into "BSD" style format, which looks like:

#include "list.h"
main(C cArg, SZ rgszArg[])
{
    I iNode;
    I cNodes = atoi(rgszArg[1]);
    I cNodesToSkip = atoi(rgszArg[2]);
    PNODE pnodeT;
    PNODE pnodeCur;
    InitNodes(cNodes);
    for (iNode = 2, pnodeCur = PnodeNew(1); iNode <= cNodes ; iNode++)
    {
        pnodeT = PnodeNew(iNode);
        InsertNext(pnodeCur, pnodeT);
        pnodeCur = pnodeT;
    }
    while (pnodeCur != PnodeNext(x))
    {
        for (iNode = 1; iNode < cNodesToSkip ; iNode++)
        {
            pnodeCur = PnodeNext(pnodeCur);
        }
        FreeNode(PnodeDeleteNext(pnodeCur));
    }
    printf("%d\n", Item(nodeCur));
}

I chose one of the variants that I personally find most attractive, any one of them could work. The thing about this example is that there are no comments.  That's reasonable given that it's an example from a textbook, and as such the words in the textbook that accompany the example suffice as the documentation.  But all code begs to be documented, so lets see what happens.

When you look at documenting a function, there are a couple of things to keep in mind when considering style (there are other important things, like the relevance of the comments, etc, but this post's about style).

The first question that comes to mind, almost immediately is "What commenting form should you use?"  For C/C++, there are a number of options.

First off, C and C++ support two different styles of comment.  The first is the traditional  /* */ style of comments.  And there's the C++ style // comment.  Each of the two comments have different flavors.

When I was in college, I loved using some of the options that /* */ comments enabled.  My code was just full of things like:

         :
         :
    }

    /*************************************************************/
    /*                                                           */
    /*                                                           */
    /*   Loop through the nodes looking for the current node.    */
    /*                                                           */
    /*                                                           */
    /*************************************************************/
    while (pnodeCur != PnodeNext(x))
    {
        for (iNode = 1; iNode < cNodesToSkip ; iNode++)
         :
         :
 

I also had this thing about putting a comment on every line, especially for assignments.  And every comment lined up on column 40 if possible.

         :
         :
    }

    /*************************************************************/
    /*                                                           */
    /*                                                           */
    /*   Loop through the nodes looking for the current node.    */
    /*                                                           */
    /*                                                           */
    /*************************************************************/
    while (pnodeCur != PnodeNext(x))    // Keep looping until PnodeNext == pnodeCur
    {
        for (iNode = 1; iNode < cNodesToSkip ; iNode++) // Skip through cNodesToSkip nodes
         :                             // A comment for this line.
         :                             // A comment for that line.
 

When I look back at my old code, the code screams "1980's", just like the hairdos of "A Flock of Seagulls" does.  But there are times when big honking block comments like that are appropriate, like when you're pointing out something that's REALLY important - like when you're about to do something that would cause Raymond Chen to swear like a sailor when he runs into your application.

Also, while I'm on the subject of comments on every line.  There ARE situations where you want a comment on every line.  For example, if you're ever forced to write assembly language (for any assembly language), you really MUST comment every line.  It's far too easy to forget which registers are supposed to contain what contents, so adding comments on every line is essential to understanding the code.

/* */ comments have the disadvantage that they take up a fair amount of space.  This means that they increase the effective line length.  // comments don't have this problem.  On the other hand, /* */ comments stand out more.

Personally I feel that both comment styles should be used.  One fairly effective form that I've seen is:

         :
         :
    }
    /*
     * Loop through the nodes until you hit the current node.
     */
    while (pnodeCur != PnodeNext(x))
    {
        for (iNode = 1; iNode < cNodesToSkip ; iNode++)
        {
            pnodeCur = PnodeNext(pnodeCur);  // Skip to the next node.
        }
         :
         :
 

Of course that's not very different from:

         :
         :
    }

    // Loop through the nodes until you hit the current node.
    while (pnodeCur != PnodeNext(x))
    {
        for (iNode = 1; iNode < cNodesToSkip ; iNode++)
        {
            pnodeCur = PnodeNext(pnodeCur);  // Skip to the next node.
        }
         :
         :
 

On the other hand, the first form (/**/) stands out more, simply because it has more white space.  You can achieve the same whitespace effect with // comments:

         :
         :
    }
    //
    // Loop through the nodes until you hit the current node.
    //
    while (pnodeCur != PnodeNext(x))
    {
        for (iNode = 1; iNode < cNodesToSkip ; iNode++)
        {
            pnodeCur = PnodeNext(pnodeCur);  // Skip to the next node.
        }
         :
         :
Another aspect of commenting style is the effective line length.  When I learned to program, I learned on 80 character wide terminals, which imposed a fairly strict limit on the number of columns in the code - anything longer than 80 characters was not OK.  Nowadays, that isn't as important, but it's still important to limit source code lines to a reasonable number, somewhere around 100 works for me, but of course your experience might be different.

The other major thing about comments is the content of the comments.  Comments can be the bane of a maintainers existence or their savior, depending on the comment.

I don't know how many times I've come across the following and cringed:

    //
    // Increment i.
    //
    i = i + 1;
 

Yes, the line's commented, but how useful is the comment?  Especially since it's a block comment (block comments have more importance than in-line comments by virtue of the amount of real estate they occupy - the very size of the comment gives it weight.  In general, you want to use in-line comments for little things and block comments for the big stuff.  I almost never use in-line comments, I prefer to use block comments at the appropriate locations and let the code speak for itself.

I've mentioned white space as an aspect of style before, but this is the time to bring it up.  Consider the example routine with some white space added to stretch the code out a bit:

#include "list.h"
main(C cArg, SZ rgszArg[])
{
    I iNode;
    I cNodes = atoi(rgszArg[1]);
    I cNodesToSkip = atoi(rgszArg[2]);
    PNODE pnodeT;
    PNODE pnodeCur;

    InitNodes(cNodes);

    for (iNode = 2, pnodeCur = PnodeNew(1); iNode <= cNodes ; iNode++)
    {
        pnodeT = PnodeNew(iNode);
        InsertNext(pnodeCur, pnodeT);
        pnodeCur = pnodeT;
    }

    while (pnodeCur != PnodeNext(x))
    {
        for (iNode = 1; iNode < cNodesToSkip ; iNode++)
        {
            pnodeCur = PnodeNext(pnodeCur);
        }
        FreeNode(PnodeDeleteNext(pnodeCur));
    }

    printf("%d\n", Item(nodeCur));
}

It's not a major change, but to me, by just inserting 4 empty lines, the code's been made clearer. 

The other aspect of white space is intra-expression white space.  This is one of the aspects of style that tends to get religious.  I've seen people who do:

    for ( iNode = 2 , pnodeCur = PnodeNew( 1 ) ; iNode <= cNodes ; iNode ++ )
And others who prefer:

    for (iNode=2,pnodeCur=PnodeNew(1);iNode<=cNodes;iNode++)
Or:

    for (iNode=2, pnodeCur=PnodeNew(1); iNode<=cNodes; iNode++)
Or:

    for ( iNode=2, pnodeCur=PnodeNew(1) ; iNode<=cNodes ; iNode++ )
The reality is that it doesn't really matter which form you use, as long as you're consistent.

Lets see what happens to the sample routine if you insert some block comments...

#include "list.h"
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));
}

To me, that's a huge improvement.  By stretching out the code and adding some comments, it's already starting to look better.

Again, just for grins, here's how it would look in my "1980's style":

#include "list.h"
main(C cArg, SZ rgszArg[])
{
    I iNode;                              // Node index.
    I cNodes = atoi(rgszArg[1]);          // Set the number of nodes
    I cNodesToSkip = atoi(rgszArg[2]);    // and the nodes to skip.
    PNODE pnodeT;                         // Declare a temporary node.
    PNODE pnodeCur;                       // And the current node.

    InitNodes(cNodes);                    // Initialize the nodes database
                                          // with cNodes elements

    /*************************************************************/
    /*                                                           */
    /*                                                           */
    /*               Create a list of cNodes nodes.              */
    /*                                                           */
    /*                                                           */
    /*************************************************************/
    for (iNode = 2, pnodeCur = PnodeNew(1); iNode <= cNodes ; iNode++)
    {
        pnodeT = PnodeNew(iNode);        // Allocate a new node.
        InsertNext(pnodeCur, pnodeT);    // Insert it after the current node
        pnodeCur = pnodeT;               // Current = New.
    }

    /*************************************************************/
    /*                                                           */
    /*                                                           */
    /*   Walk the list of nodes, freeing the node that occurs    */
    /*   at every cNodesToSkip nodes in the list.                */
    /*                                                           */
    /*                                                           */
    /*************************************************************/
    while (pnodeCur != PnodeNext(x))    // While not at the current node
    {
        for (iNode = 1; iNode < cNodesToSkip ; iNode++)
        {
            pnodeCur = PnodeNext(pnodeCur); // Current = current->Next
        }
        FreeNode(PnodeDeleteNext(pnodeCur)); // Free current->Next
    }

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

Yech.

Tomorrow: function and file headers.

  • I can see one use for the 1980's brand, which still seems to be fairly common today: The tops of files, to hold all the general information, authorship, and 'mission statement'.
  • Hi,

    just some comments.
    // is a kinda nasty technique. Some GCC ports
    (WindRiver's Tornado is one of them) do not
    quite like them. And if you willing to
    be somewhat portable, shall note that.
    Also I doubt that there is worse than manually
    "fixing" the beauty of your code.
    I have been using "idnt.exe" (it's a free
    (even source, guess you Microsoft guys just
    loove the idea) C formatter), you can
    do almost everything with it. Specifying
    block format style, i.e. K&R or other,
    linelength, comment style, removal of
    TABs, etc.
    Forth taking a good look at, really.
    Speeds up "manufacturing" comments and prologs
    as hell, since you simply do not bother how
    disgusting your comment look like, since it'll
    be reformatted automatically.
    Nice blog, Larry anyhow! Keep up.
  • <i>To me, that's a huge improvement. By stretching out the code and adding some comments, it's already starting to look better.</i>

    Oh, I would so agree - that's the style I find most readable, and tends to be what my own code is written in. In the unlikely event we ever have to read each other's code, I think it might work quite well. ;-) After some of the stuff I've read recently, it's nice to find other people who appreciate readable code!

    As for disabling code - #if 0/#endif all the way. Much easier than comments, both for enabling and disabling, and Vim also highlights an "#if 0" region as a comment so you know it's disabled. (Vim's auto-formatting is also out of this world - nothing else I've tried comes close.)
  • Jeff, thanks for your links, exploring them :)

    Unfortunately I don't know NOTHING about .net and CLR. I even don't know what CLR stands for :) Gonna start researching it one day...
  • Ilya,
    CLR means "Common Language Runtime" - also known as the .Net framework.
  • Cool, coupla days ago I though out "Common Libraries Runtime", so I was rather close to the truth :)

    Thanks :)
  • Something that has not been mentioned is commenting styles that will work for more than just one language.

    Our development team develops (at the moment) in VB6, C (actually Microstation MDL), C++, VB.NET and C#. These languages are similar enough that we decided to create a commenting style that will suit both the developers and the respective IDE's as well as be generally the same acros the different platforms.

    What we settled on was using one-line comments (' for VB, // for c/c++/c#) in the following manner:

    ' comment describing
    ' only the next line
    <Some VB Code Here>

    ' comment describing a block of code
    ' ----------------------------------

    VB Code line1
    VB Code line2
    VB Code line3

    VB Code line4
    ' ----------------------------------

    You can do the same with //-style comments.

    Sorry for being so verbose :)
    p!
  • Back in my younger years I would use loads and loads of asterisks in my comments. Not as many as Larry, mind, but still quite a lot:
    /**********************
    * Something or Other *
    **********************/
    (although sometimes I'd use slashes on every line. I settled down after a while.)

    However, for the past six years or so I've been using text editors with syntax colouring, so my comments don't need to be so huge anymore. I tend to just do this:
    // Something or Other
    ... and my editor makes them green. The wonders of modern technology.
  • PingBack from http://woodtvstand.info/story.php?id=6623

  • PingBack from http://portablegreenhousesite.info/story.php?id=5188

  • PingBack from http://portablegreenhousesite.info/story.php?id=14811

  • PingBack from http://hairgrowthproducts.info/story.php?id=4369

  • PingBack from http://greenteafatburner.info/story.php?id=4147

Page 2 of 3 (33 items) 123