Code commenting

Bob Phillips sent me this magnificent example of quality code commenting (from MS somewhere, apparently)

Sub SelectEveryNthRow()
    ' Initialize ColsSelection equal to the number of columns in the
    ' selection.
    ColsSelection = Selection.Columns.Count

    ' Initialize RowsSelection equal to the number of rows in your
    ' selection.
    RowsSelection = Selection.Rows.Count

    ' Initialize RowsBetween equal to three.
    RowsBetween = 3

    ' Initialize Diff equal to one row less than the first row number of
    ' the selection.
    Diff = Selection.Row - 1

    ' Resize the selection to be 1 column wide and the same number of
    ' rows long as the initial selection.
    Selection.Resize(RowsSelection, 1).Select

    ' Resize the selection to be every third row and the same number of
    ' columns wide as the original selection.
    Set FinalRange = Selection. _
    Offset(RowsBetween - 1, 0).Resize(1, ColsSelection)

    ' Loop through each cell in the selection.
    For Each xCell In Selection
        ' If the row number is a multiple of 3, then . . .
        If xCell.Row Mod RowsBetween = Diff Then
            ' ...reset FinalRange to include the union of the current
            ' FinalRange and the same number of columns.
            Set FinalRange = Application.Union _
            (FinalRange, xCell.Resize(1, ColsSelection))
        ' End check.
        End If
    ' Iterate loop.
    Next xCell
    ' Select the requested cells in the range.
    FinalRange.Select
End Sub

Stunning!

Thanks Bob !

As Bob says, if ever there was an argument against commenting, this is it (Bob inserted the blank lines to make it readable)

It might be a bit arrogant, but I think you need to write your code on the understanding whoever comes later will have a reasonable level of competence. If they can’t work out what

Selection.Columns.Count

means then I’m not sure they should be poking around VBA. (or anything important really).

What do you think?

cheers

Simon

Advertisements

12 Responses to “Code commenting”

  1. jonpeltier Says:

    “If it was hard to write, it should be hard to read.” – Chip Pearson

  2. Bryan Schmidt Says:

    Expanding on the quote that jonpeltier cited from Chip Pearson, Programming a piece of software is not a linear process, unless you are building a simple, less-than-20-line, program. You are constantly debugging and adding, deleting, and changing code. Even if I never intend for anyone else to read my code, I still comment the heck out of it. Sometimes, I’ll even comment in what I was thinking, or how I came to the conclusion to do it this certain way, rather than an alternative method. I’ll comment out old processes and comment why I removed it, or if I plan to add it in later. This way, when I come back to a year later, I can pick up where I left off without rethinking a lot of the processes. And then on the other hand, if someone else wants to poke around, they won’t be helplessly lost. :D

  3. Marcus Says:

    I think there is a distinction between documenting the intent or purpose of your code such as the method behind your madness (the reason for taking a particular approach), assumptions, dependencies and so on, compared against repeating what each line of code does in your commentary.

    As Bryan noted, he comments how he decided to do it a certain way. I’ll even include URL’s to MSDN articles or sample code which impacted the design.

    This example appears to be more of a tutorial. But in true MS style, there’s no variable declarations or naming conventions.

  4. Harlan Grove Says:

    For basic procedures who needs naming conventions?

    I use naming the conventions from K&R C or the last numerical programming text I read. I’m still waiting to find a rational as opposed to religiously orthodox reason to use variables named like lArrayIndex1, lArrayIndex2 and lArrayIndex3 rather than i, j and k.

    This one’s the most offensive comment:

    ‘ If the row number is a multiple of 3, then . . .
    If xCell.Row Mod RowsBetween = Diff Then

    Go to all that trouble to abstract the number of rows in RowsBetween then put the magic number into the comment. Classic!

  5. Bob Phillips Says:

    The best part of that code is that it only works if you start the selection in row 1, any other row and it fails in its objectives. Commented to death, crap code.

    I posted this to Simon because I know his views are the extreme of what I think, that most comments are a waste of time. If ever I pick up someone else’s code I tend to strip out the comments because I can then read the code better, and experience tells me that the comments will enlighten me very little, if at all. As to my own code, yes my comments are great of course, at the time I wrote them. Going back 6 months, a year later, I am just as confused by them as I am by others … my techniques will have changed, my naming conventions, all manner of things. Far better to get cracking on the code IMO.

    And worst of all … module and procedure banners. I did a job for a major high street retailier last year and I ad to include this in every module, every procedure, giving standard banners, a a functional description, list of the procedures in the module, what called what and by what, and on. It took longer to write that stuff that people ignore than the code.

    URLs are an inteesting thought. Of course they are guaranteed to always be there!

  6. Rob Bruce Says:

    I’m willing to bet that this was originally posted as an example of how not to comment code. Do I win?

    (‘ Iterate loop. !!!!!!!! :-))

  7. mrt Says:

    The Code came from here:

    http://support.microsoft.com/kb/213438

  8. Dick Kusleika Says:

    “I’m still waiting to find a rational”

    Mine is that I can reuse variable names of different types. I can use lUser for a long, sUser for a string, and so on. I don’t know if I’m rationalizing my religious orthodoxy or if it’s really useful, but I choose to believe the latter. And I always use i,j,k for loops.

    “And worst of all”

    I did a project a couple of years ago and used procedure-level headers (for the first time) that described the purpose, the arguments, and other seemingly relevant data. I found these very useful to get up to speed on the flow when I revisited the code, but didn’t find any of my inline comments useful. I can’t say if it was a time saver, though. I think it was about a wash. If I ever have to look at the code again, watch out. Productivity through the roof.

  9. Simon Says:

    Thanks mrt

    Sorry Rob it looks like you were not successful this time…

    I’d rather see zero comments, everything scoped as tightly as possible, everything typed sensibly, reasonable names, and decent error handling. And all that would give better code for less effort than this ‘ministry of the bleedin obvious’ tat.

    nooBs are going to be wondering where did ColsSelection etc come from? and what is it? part of Excel?

    I tend to only comment stuff that doesn’t work as it should or that is a workaround. But when I have functions called ‘WorkAroundForIEBogShite(‘ I think the intent is pretty clear!

    For teaching, copious comments are fair enough, especially if combined with good practice.

    I wouldn’t fancy commenting every mod and proc, or maintaining something like that (maintain the code I mean, I would ignore the comments of course). Commenting callers is a workaround for having a crap editor that can’t list callers, and being unaware of MZ Tools.

  10. Simon Says:

    Dick good point on the module stuff.
    i do try and put a couple of sentences on what each mod or class is for.

  11. Stephane Rodriguez Says:

    I belong to the school of thought that comments only mean unfinished code.

    With the module name, class name, procedure name and variable name, there is enough opportunities to explain what the code is trying to achieve without the need of a single comment. And this does not preclude the fact that procedure split in more granual ones adds more explanations whenever really necessary (in addition to being geared towards factoring and reuse).

  12. Brian Says:

    ‘ Initialize RowsBetween equal to three.
    RowsBetween = 3

    Ha, the comment above is more confusing than the simple line of code:)

Leave a Reply

Please log in using one of these methods to post your comment:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s


%d bloggers like this: