Commenting code is a good thing! It helps other programmers to read your code and as such reduces the chance for bugs. WRONG! Commenting code CAN be a good thing and it CAN help others (and yourself) to read your code, or it can do quite the opposite... Just for clarity, this article talks about comments in code, not XML comments (that document code and give you some IntelliSense).
What you first need to understand is that each written comment is a line of text that has to be maintained. After you alter a piece of code that is commented, you will have to read the comments and possibly alter them if they don't hold true anymore. In my experience, this doesn't happen to often. Fixes are made quickly and time is sparse, too sparse to rewrite any comments! The result is often code littered with unclear comments that just don't explain the code it is supposed to clear up...
Too often have I seen comments that didn't make sense, that didn't add value to the code, that were unclear, or that were just plain wrong. So let's take a look at how we can reduce comments.
int i = myControl.SelectedIndex();
Now, there are two things wrong in this code. First of all, the variable name
i is unclear. What does
i represent? Apparently, the coder thought so too, because he commented on what
i is. So with that comment, our code is understandable again, right? No. Even without the comment, we could see that
i is the
myControl, simply because it stands right there. So does that make it right to simply delete the comment and our code will be clear again? Still no. What if
i is used later in our code? Let's say we have the following line of code a few lines ahead.
So what is
i now? We must read a few lines back to see what
i represents. Or we could write another neat comment to tell us that
i is the selected index! No, no no! Commenting isn't the answer! Writing clear code is!
So let's rewrite the first bit of code so that it is clear what
i represents and which also eliminates the need for any comments.
int selectedIndex = myControl.SelectedIndex();
By properly naming our variables, we now know the purpose of
i and we eliminated the need for comments.
Let's take a look at another example of wrong commenting.
List<int> productIds = new List<int>();
While this comment certainly adds something to the code, being what a
List<T> is and how it works, I highly discourage you to use these kind of comments in your code. Why? First of all,
Generics are part of the .NET Framework and are very well documented on MSDN and other websites. I hope the person that is going to work on your code knows how to use these
Classes or at least how to find out. Whatever the case, it is not your job to document the framework you are using. Also, what if new requirements or insights require not a list of product IDs, but a dictionary of product IDs and the
Product Classes they represent? You would have written all those comments for nothing and the person who is going to change the code needs a lot of time to go through your comments and may even feel obliged to document the now used
Dictionary(int, Product) in much the same way! Keep in mind that we are writing code, not how-to books.
So how about the next example?
The code is easy to read. It looks like the coder thought about his variable- and function names. The comment looks clear and to the point too. Still I say nay. What does the comment add to the code? Absolutely nothing. It simply switches the words
SaveToDatabase (and adds some spaces). The comment is just a grammatically correct version of our code. That might help your little sister to follow our code, but your little sister isn't going to maintain this. Programmers are going to maintain it, and if a programmer cannot guess what
customer.SaveToDatabase does he might as well find another career right away!
Now here's an interesting one. It really shows how neglected comments are and why you should use them as sparingly as possible. Someone copy-pasted the code above. Apparently, the code for
product looked much the same, but wasn't abstracted out enough to re-use existing code, hence copy-paste was used instead. This was the result.
While copy-pasting is a sin in its own right, let's focus on the comment here. It wasn't changed, it probably wasn't even read by the programmer who pasted this! Everything works, everything compiles, but we have this big lying comment staring at us. What happened here? Did the programmer mean to save the customer? Did the customer get saved here at some point in time? It's probably best to just delete this comment. And had we never written it in the first place, that might've saved us some time, effort and confusion! Mind you, these are real-world examples I found in production code!
And while we're on the subject of commenting the obvious, how about commenting the non-obvious in a non-obvious manner?
string helper = null;
Yeah, that comment really made a difference... We still don't know what
helper is or does! Reading probably about twenty lines further I found this.
helper = "Some pretty long message";
That's all it did... Now we could have done all of this without the
helper variable, but if we were using a variable to hold our
string message, perhaps for readability, we could've at least named it something like
message or even
msg, any of those would've been clearer than
helper. Second, we could've kept the scope of the variable a lot smaller. Keeping scope of variables as small as possible is pretty good practice and helps for better readable code. So instead of declaring it twenty lines earlier, we could've declared it inside the
if statement. Let's look at the new code.
string message = "Some pretty long message";
Much better, no?
Another sin I have found is keeping a change log in code. Consider the next example.
Apparently, the customer demanded that something did or did not happen if
true. When we are reading the code, do we really care this was ticket #666, that I fixed (or broke) it and that it was on 28 september 2012? I think not. What's also a problem is that many fixes are not made in a few lines in a single file. Often, we need to change a few
Classes, maybe add some code and even delete code. Do we need to put the ticket comment in each file? How are we going to comment deleted code? Now if we do want to see who coded what, there are plenty of tools for us to track and find out, one being Source Control tools. For example, I use Tortoise SVN or Git which both let you see exactly which user wrote what line of code or when a file was edited, what was edited and by whom. Not a single comment (or even a whole bookwork of comments) can beat that!
Something I also find very often is commented out code. Somehow programmers are afraid to delete code. They feel they've spent some good time writing their code and simply deleting it would hurt the code's feelings or something. I've even heard excuses like "I'm not sure if this code is still being used so I'll just comment it out instead of delete it." What!? There is no difference for the compiler! So if the application is going to work with the code commented out, it will work with the code deleted as well!
I literally find entire code files, hundreds of lines of code, commented out... How does this help anyone? If, for some reason, and it happens, you've been a little to enthusiastic with the delete button there's always Source Control to get it back.
This kind of comment is very often related to the kind of comment I describe above, keeping change logs in code. Commented out code is something like a change log, a log of how your code has changed. Just not a very clear or structured one. So just use Source Control!
I want to present to you a last example of a badly written comment.
int omega =
int alpha =
The comment explains that we need
omega to achieve world domination. You might have wondered why you needed
omega, now you know. However, when reading this comment, the first thing that comes to mind is what is
alpha and what is
omega? And why is the difficult math that follows not commented? Let's rewrite that.
int omega =
int alpha =
Now this is actually somewhat of a gray area. You and I don't know what
omega are, but perhaps it is common knowledge (or assumed common knowledge) at Evil & Co., the company that concocted this evil plan and wrote the code. In that case, it would be time consuming and not adding to the code. This example shows that a 'good' comment can be highly subjective. Likewise, if everyone working at Evil & Co. has a masters degree in math, it might not be necessary to explain the difficult math.
So I make it sound like comments are the root of all evil. Are there no good comments? Sure there are, let's consider them a necessary evil.
string pattern = @"\bp(\w*)n\b"
Regex regex = new Regex(pattern, RegexOptions.IgnoreCase);
Now that might save any future programmer a lot of time! I don't know about you, but I don't use regular expressions all that often and finding out what the pattern matches exactly would have me search the internet for at least a few minutes. When the pattern grows larger, the gain from the comment grows too.
Let's look at more helpful, even crucial, comments.
Now an empty
catch block might not be deserving of a beauty prize, but at least you know why it is there. More importantly, you might have removed the empty
catch block because calling code would have sufficiently caught and handled any Exception anyway. However, now that the comment is there, you know the empty
catch block is there by design and you better not touch it!
In conclusion, I can only say that writing good comments is difficult and what might be a good comment in one situation might be bad in another. However, if your code is littered with comments that don't clear up any code people might learn to ignore your comments and miss the important ones. On top of that, comments are lines of text which have to be maintained just like any other code. As such, comments deserve the same thought as your regular code. If a comment isn't worth writing, then don't write it! If it is, think twice, then think on what you are going to write and if you are still convinced the comment is absolutely necessary for future generations to maintain your code, go for it.
"Don't comment bad code-rewrite it." -- (Brian W. Kernighan and P.J. Plaugher)
- 28th September, 2012: Initial version