Question

We all know that commenting our code is an important part of coding style for making our code understandable to the next person who comes along, or even ourselves in 6 months or so.

However, sometimes a comment just doesn't cut the mustard. I'm not talking about obvious jokes or vented frustraton, I'm talking about comments that appear to be making an attempt at explanation, but do it so poorly they might as well not be there. Comments that are too short, are too cryptic, or are just plain wrong.

As a cautonary tale, could you share something you've seen that was really just that bad, and if it's not obvious, show the code it was referring to and point out what's wrong with it? What should have gone in there instead?

See also:

Was it helpful?

Solution

Just the typical Comp Sci 101 type comments:

$i = 0; //set i to 0

$i++; //use sneaky trick to add 1 to i!

if ($i==$j) { // I made sure to use == rather than = here to avoid a bug

That sort of thing.

OTHER TIPS

Unfilled javadoc boilerplate comments are particularly useless. They consume a lot of screen real estate without contributing anything useful. And the worst part is that where one such comment appears, hundreds of others are surely lurking behind.

/**
 * Method declaration
 *
 *
 * @param table
 * @param row
 *
 * @throws SQLException
 */
void addTransactionDelete(Table table, Object row[]) throws SQLException {

I've found myself writing this little gem before:

//@TODO: Rewrite this, it sucks. Seriously.

Usually it's a good sign that I've reached the end of my coding session for the night.

// remember to comment code

wtf? :D

Something like this:

// This method takes two integer values and adds them together via the built-in
// .NET functionality. It would be possible to code the arithmetic function
// by hand, but since .NET provides it, that would be a waste of time
private int Add(int i, int j) // i is the first value, j is the second value
{
    // add the numbers together using the .NET "+" operator
    int z = i + j;

    // return the value to the calling function
    // return z;

    // this code was updated to simplify the return statement, eliminating the need
    // for a separate variable.
    // this statement performs the add functionality using the + operator on the two
    // parameter values, and then returns the result to the calling function
    return i + j;
}

And so on.

Every comment that just repeats what the code says is useless. Comments should not tell me what the code does. If I don't know the programming language well enough, to understand what's going on by just reading the code, I should not be reading that code at all. Comments like

// Increase i by one
i++;

are completely useless. I see that i is increased by one, that is what the code says, I don't need a comment for that! Comments should be used to explain why something is done (in case it is far from being obvious) or why something is done that way and not any other way (so I can understand certain design decisions another programmer made that are by far not obvious at once). Further comments are useful to explain tricky code, where it is absolutely not possible to determine what's going on by having a quick look at the code (e.g. there are tricky algorithms to count the number of bits set in a number; if you don't know what this code does, you have no chance of guessing what goes on there).

Thread.Sleep(1000); // this will fix .NET's crappy threading implementation

I once worked on a project with a strange C compiler. It gave an error on a valid piece of code unless a comment was inserted between two statements. So I changed the comment to:

// Do not remove this comment else compilation will fail.

And it worked great.

I don't believe it. I came into this question after it had 22 answers, and no one pointed out the least possibly useful type of comment:

comments that are wrong.

It's bad enough that people write superfluous comments that get in the way of understanding code, but when someone writes a detailed comment explaining how something works, and it's either wrong in the first place, or wrong after the code was changed without changing the comment (much more likely scenario), that is definitely the worst kind of comment.

GhostDoc comes up with some pretty interesting ones on its own.

/// <summary>
/// Toes the foo.
/// </summary>
/// <returns></returns>
public Foo ToFoo()
// secret sauce
// Don't know why we have to do this
try
{
...some code...
}
catch
{
// Just don't crash, it wasn't that important anyway.
}

*sigh

Came across a file once. Thousands of lines of code, most of it quite horrendous. Badly named variables, tricky conditionals on loops and one comment buried in the middle of the file.


   /* Hmmm. A bit tricky. */
//' OOOO oooo that smell!! Can't you smell that smell!??!??!!!!11!??/!!!!!1!!!!!!1

If Not Me.CurrentMenuItem.Parent Is Nothing Then
    For Each childMenuItem As MenuItem In aMenuItem.Children
        do something
    Next

    If Not Me.CurrentMenuItem.Parent.Parent Is Nothing Then
        //'item is at least a grand child
        For Each childMenuItem As MenuItem In aMenuItem.Children
            For Each grandchildMenuItem As MenuItem In childMenuItem.Children
                do something
            Next
        Next

        If Not Me.CurrentMenuItem.Parent.Parent.Parent Is Nothing Then
            //'item is at least a grand grand child
            For Each childMenuItem As MenuItem In aMenuItem.Children
                For Each grandchildMenuItem As MenuItem In childMenuItem.Children
                    For Each grandgrandchildMenuItem As MenuItem In grandchildMenuItem.Children
                        do something
                    Next
                Next
            Next

        End If
    End If
End If

Default comments inserted by IDEs.

The last project I worked on which used WebSphere Application Developer had plenty of maintenance developers and contractors who didn't seem to be bothered by the hundreds, if not thousands of Java classes which contained the likes of this:

/**
 * @author SomeUserWhoShouldKnowBetter
 *
 * To change this generated comment edit the template variable "typecomment":
 * Window>Preferences>Java>Templates.
 * To enable and disable the creation of type comments go to
 * Window>Preferences>Java>Code Generation.
 */

There was always that split-second between thinking you'd actually found a well-commented source file and realising that, yup, it's another default comment, which forced you to use SWEAR_WORD_OF_CHOICE.

I saw this comment yesterday in a C# app:

//TODO: Remove this comment.

My favorite all-time comment.

/* our second do loop */
do {

Whoever wrote it - you know who you are.

a very large database engine project in C many many years ago - thousands of lines of code with short and misspelled variable names, and no comments... until way deep in nested if-conditions several thousands of lines into the module the following comment appeared:

//if you get here then you really f**ked

by that time, i think we knew that already!

In a huge VB5 application

dim J
J = 0 'magic
J = J 'more magic
for J=1 to 100
...do stuff...

The reference is obviously THIS ... and yes, the application without those two lines fails at runtime with an unknown error code. We still don't know why.

Taken from one of my blog posts:

In the process of cleaning up some of the source code for one of the projects I manage, I came across the following comments:

/*
   MAB 08-05-2004: Who wrote this routine? When did they do it? Who should 
   I call if I have questions about it? It's worth it to have a good header
   here. It should helps to set context, it should identify the author 
   (hero or culprit!), including contact information, so that anyone who has
   questions can call or email. It's useful to have the date noted, and a 
   brief statement of intention. On the other hand, this isn't meant to be 
   busy work; it's meant to make maintenance easier--so don't go overboard.

   One other good reason to put your name on it: take credit! This is your
   craft
*/

and then a little further down:

#include "xxxMsg.h" // xxx messages
/*
   MAB 08-05-2004: With respect to the comment above, I gathered that
   from the filename. I think I need either more or less here. For one
   thing, xxxMsg.h is automatically generated from the .mc file. That might
   be interesting information. Another thing is that xxxMsg.h should NOT be
   added to source control, because it's auto-generated. Alternatively, 
   don't bother with a comment at all.
*/

and then yet again:

/*
   MAB 08-05-2004: Defining a keyword?? This seems problemmatic [sic],
   in principle if not in practice. Is this a common idiom? 
*/

AHHHRRRGGHHH Just found this in some ancient code, bet the guy thought he was pretty funny

private
  //PRIVATE means PRIVATE so no comments for you
  function LoadIt(IntID: Integer): Integer;

The worst comment is one that gives a wrong explanation of what the code does. That is worse than no comment at all.

I've seen this kind of thing in code with way too many comments (that shouldn't be there because the code is clear enough on its own), and it happens mostly when the code is updated (refactored, modified, etc.) but the comments aren't updated along with it.

A good rule of thumb is: only write comments to explain why code is doing something, not what it does.

Would definitely have to be comments that stand in place of error handling.

if(some_condition){
    do_stuff();
}
else{
    //An error occurred!
}

I just found this one, written on the line before a commented-out line of code:

//This causes a crash for some reason. I know the real reason but it doesn't fit on this line.

100k LOC application that was ported from vb6 to vb.net. It looks as though a previous developer had put a comment header on one method and then copied and pasted the exact comment onto every method he wrote from then on. Hundreds of methods and each one incorrectly commented...

When i first saw it i laughed... 6 months later the joke is wearing thin.

This is an absolutely real example from a database trigger:

/******************************************************************************
   NAME:       (repeat the trigger name)
   PURPOSE:    To perform work as each row is inserted or updated.
   REVISIONS:
   Ver        Date        Author           Description
   ---------  ----------  ---------------  ------------------------------------
   1.0        27.6.2000             1. Created this trigger.
   PARAMETERS:
   INPUT:
   OUTPUT:
   RETURNED VALUE:
   CALLED BY:
   CALLS:
   EXAMPLE USE:
   ASSUMPTIONS:
   LIMITATIONS:
   ALGORITHM:
   NOTES:
******************************************************************************/
/** function header comments required to pass checkstyle */

The two most unhelpful comments I've ever seen...

try
{
  ...
}
catch
{
  // TODO: something catchy
}

I posted this one at the Daily WTF also, so I'll trim it to just the comment...

  // TODO: The following if block should be reduced to one return statememt:
  // return Regex.IsMatch(strTest, NAME_CHARS);
  if (!Regex.IsMatch(strTest, NAME_CHARS))
    return false;
  else
    return true;

One I've never found very helpful:

<!--- Lasciate ogne speranza, voi ch'intrate --->
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top