Comments

Recently, while refactoring an old mess of code, I came across the following:

   1: ArrayList result = new ArrayList(dictionary.Count);
   2:  
   3: // Loop through the dictionary and add it to the ArrayList
   4: for(int iNdx = 0; iNdx < dictionary.Count; iNdx++)
   5: {
   6:   DictionaryItem dictionaryItemValue = dictionary[iNdx];
   7:   result.Add(dictionaryItemValue);
   8: }

 

What is wrong with this code? For me, I see following:

  • ArrayList when we should be using a List<>
  • "for" when we should be using "foreach"
  • A difficult to read loop counter
  • An ugly temporary variable when we should be using none
  • If we used List<>, we would be able to use "AddRange" and bypass the loop entirely.

BUT, none of these are what I find most objectionable. The thing that smells the worst to me is the comment. Yup, that line above the "for" loop. What is it there for? What is its purpose in life? If somebody were to come along and read this code, the comment would only waste their time. It is obvious. Duh.

Comments, by themselves, are a code smell. They muddy the code in a way that makes it harder to read and less maintainable. In fact, I have this poster hanging in my cube at work to remind me in case I am feeling lazy:

To illustrate my point, consider the following inane example: What if I fixed everything about the code that I mentioned preiously:

   1: List<DictionaryItem> result = new List<DictionaryItem>();
   2:  
   3: // Loop through the dictionary and add it to the ArrayList
   4: result.AddRange(dictionary);

 

Now I have a comment that doesn’t even make sense anymore. You may say "If you made that change, you would change the comment as well." This is false. I am VERY likely to ignore the comment and leave the bad comment in place. Why?

  1. The comment was useless to begin with, so I ignored it.
  2. Most single-line comments are useless, so I ignore it.
  3. The compiler won’t tell you that the comment doesn’t make sense anymore.
  4. I might be using a tool like ReSharper where most the work is done for me. It ignores comments too.

IT HAPPENS ALL THE TIME.

As far as I am concerned, comments are usually a cheap replacement for readable code. It is easy to slap a comment in and walk away, but it takes thought and effort to make the code readable without the comment.

Now, I am the first to admit… my code readability needs work. More specifically, the way I write code needs work to be readable by others. It is kind of like when I cook (my biggest non-tech hobby). I almost always make food that tastes good… to me. Making it taste as good for others is the thing I am constantly trying to make better.

Here are the categories of comments that I see on a regular basis:

Incorrect Comments

These comments creep int a code base over time as as the code is refactored. If you comment often, then it is inevitable that many of your comments will morph into incorrect comments. These should be eliminated whenever they are found, as they do nothing but confuse the reader. If the code needs a comment, consider refactoring to eliminate the need.

Obvious Comments

So often, a comment is telling you exactly what the code is doing…

   1: // Check For Null
   2: if(item == null)

These comments will quickly become incorrect comments because they are so useless that nobody pays attention to them anyways. They should be removed whenever they are found.

Comments that Replace Readable Code

It takes a lot more effort and creativity to write readable code than it does to comment your code. The problem, again, is that code often changes while comments are often not updated. When this happens, you are left with hard-to-read code with incorrect comments. The author should have encapsulated the code into a concise method that does one thing — the thing that the comment says that it does.

When you see something like:

   1: List<string> keyData = new List<string>();
   2:  
   3: // If the file version is old, then parse the old keyed data from the list
   4: if (string.IsNullOrEmpty(v) ||v.StartsWith("1.") ||v.StartsWith("2.") && v.EndsWith("b")))
   5: {
   6:     foreach (string stuff in ReadStuffFromFile(fh))
   7:     {
   8:         if (stuff.Contains(_specialKey))
   9:         {
  10:             string keyItem = stuff.Split(_specialKey.ToCharArray())[1];
  11:             keyData.Add(keyItem.Substring(3, 8));
  12:         }
  13:     }
  14: }

It is so much easier to read if you bust the condition and the block into separate methods:

   1: if (FileVersionIsLegacy(v))
   2:     keyData = ParseLegacyData(fh);

Comments that excuse bad code

I am fairly guilty of this one. Comments that say "I know this is bad form, but it is more performant to so it this way" or "Working around a Microsoft bug" are sometimes excusable. Instead, though, a colleague of mine once suggested that I write a method that is named something like HandleListPerformantly() or WorkAroundMicrosoftDisplayBug_3334495() instead. You at least make it obvious to everyone involved what you are doing. Why write a comment when you can use code?

Multi-line comments describing complex behavior

The answer to my last question might be "Because it takes me several lines to describe why I am doing something". I have a hard time arguing with this one. I remember reading in someone’s blog recently (I can’t remember who, or else I would cite it) Jeff Atwood wrote a post once that code is the "how" and comments are the "why". This is a case where I agree with that statement. I like to stick to the general rule that if a comment is 2 lines or less, I can probably re-write it to be more readable. If it is longer, then my comment is probably useful. Unfortunately, large comment blocks fall victim to accuracy over time even quicker, due to the fact that the nobody who knows the code tends to read those long comments, so they tend to overlook them when they refactor. Consider ways to avoid these blocks as well.

Auto-Documentation comments

I am bringing up this type of comment because every time I have an argument with a staunch comment supporter, they always bring up this case. I will say it now: this doesn’t count. It doesn’t count because documentation in comments has its own language and stucture. They aren’t really comments, but inline documentation meant for external consumers. They get parsed and compiled with a tool, reviewed by a human before delivery and they tend to stay up to date as well as any other documentation method. When I say "Don’t write comments in your code", I am not talking about this type of comment.

I may have missed a few prototypes of comments, but for the most part, I think I got them all. I know this is a controversial topic, but the more I argue it, the more I believe I am right on this. The anti-comment sentiment is not new, and it is certainly not unique to me, but I wanted to document my thoughts on it anyway, as it has changed about 179 degrees in the last 3 years. My blog is inteneded to document my thoughts and attitudes about my craft.

Leave a Reply