Recently, while refactoring an old mess of code, I came across the following:
ArrayList result = new ArrayList(dictionary.Count);What is wrong with this code? For me, I see following:
// Loop through the dictionary and add it to the ArrayList
for(int iNdx = 0; iNdx < dictionary.Count; iNdx++)
{
DictionaryItem dictionaryItemValue = dictionary[iNdx];
result.Add(dictionaryItemValue);
}
- 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.
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:
List<DictionaryItem> result = new List<DictionaryItem>();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?
// Loop through the dictionary and add it to the ArrayList
result.AddRange(dictionary);
- The comment was useless to begin with, so I ignored it.
- Most single-line comments are useless, so I ignore it.
- The compiler won't tell you that the comment doesn't make sense anymore.
- I might be using a tool like ReSharper where most the work is done for me. It ignores comments too.
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...
// Check For NullThese 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.
if(item == null)
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:
List<string> keyData = new List<string>();It is so much easier to read if you bust the condition and the block into separate methods:
// If the file version is old, then parse the old keyed data from the list
if (string.IsNullOrEmpty(v) ||
v.StartsWith("1.") ||
v.StartsWith("2.") && v.EndsWith("b")))
{
foreach (string stuff in ReadStuffFromFile(fh))
{
if (stuff.Contains(_specialKey))
{
string keyItem = stuff.Split(_specialKey.ToCharArray())[1];
keyData.Add(keyItem.Substring(3, 8));
}
}
}
if (FileVersionIsLegacy(v))Comments that excuse bad code
keyData = ParseLegacyData(fh);
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.
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.
15 comments:
I ought to roundhouse kick you in the face, Chuck Norris style. Comments are never worthless. Even if they seem worthless, they're a good habit. If the code changes and the comment doesn't, that's the fault of the person who made the code change.
Wow, such hostility... I agree that if the code changes and the comment does not, that the person is at fault. It doesn't change the fact that it happened... and it happens all the time.
I used to feel the way you do, but I have changed my views after running into poor comments time and time again. I disagree that comments are a good habit. Instead, writing readable code is the good habit to get into.
If you can make your point in a comment, you can almost always make the same point in readable code. The code is live... comments are not.
actually, writing readable code and writing good comments are both good habits. i can read a sentence and know plainly what you're trying to accomplish. i have to figure out what your code is trying to accomplish, and those might be totally different things. readable code to one person is unreadable code to someone else.
I suppose that this is where we disagree. I believe that most comments supporting readable code are redundant. They have a strong tendency to fall victim to entropy much more than the code does. I'd much rather spend my time writing tests that reinforce the expected behavior than write comments that are otherwise redundant. IF you keep your methods very small, concise and well-named, comments only muddy the readability.
yeah i completely disagree. any IDE out there worth its salt will support code folding, and most do comment folding with touch of a button.
i know how you feel man. i've been there. i've had to trudge thru writing comments as well. but i've also had to go thru someone else's code. someone else who thought that their code would be readable by itself. and that can be a nightmare. save yourself, and those after you, the trouble just write the comments
I don't really like code folding. I think Jeff Atwood sums it up well: http://tinyurl.com/6escw6
It is one thing to think that my code is readable, and another thing if I write my code with comments in mind. If I can write my comments in the code directly, wouldn't that be better than comments? I haven't come to this conclusion based on my own opinion on the readability of my code. It has come from practice and studying. I urge others to do the same.
In fact, I challenge you to try my way... just for a week. If you come across code that you would normally want to comment, figure out a way to write it without comments, but still get the point across. Reduce the complexity of your code down to smaller, bite-sized morsels and put your comments directly in the method names.
You might not walk away agreeing with me, but you might walk away with some new strategies for more readable code.
man, i've tried that way; it doesn't work! i've been coding PHP for about 10 years, and writing your code that way only serves to make it readable to yourself. Another person who comes along might not be thinking the same thing as you. nothing beats a short comment describing what is trying to be accomplished.
atwood's example is really only for people who use code folding as a crutch, and where code folding is the default (such as the #region examples). The editor I used has comment and code folding as an option. You can use it or not. I find it nice to fold up a method when I'm done with it so that I don't have to scroll over it a million times. but if you like to view the same code over and over, that's fine too. to me, it makes my code MORE readable b/c i can see all of my classes methods very easily, and just read the comments about their arguments, return values, etc.
comments have their place. obviously, you don't need to write 3 pages worth of comments to get your point across. just like anything, they can be overused. but more often than not, i find them UNDER used badly.
I suppose I would ask you a question... Are you constantly refactoring your code?
If the answer is yes, how do you ensure that your comments stay in sync with your code? I know that I am not smart enough to keep this up.
How do you ensure that your co-workers keep your comments in sync with your code? None of my (very talented) co-workers are smart enough to keep this up.
My compiler can't help me...
So what do you do to make sure this happens?
If the answer is no, then you are coding in a different world than I am, and I can't argue with you further.
It's not a matter of being smart, it's a matter of being diligent. If the code changes, the comments with it should change. this can easily be enforced through code reviews.
So how do you do your code reviews? Do you code review 100% of your code? Or do you pair program?
If you are doing one of those things, then the reviewer can help you make sure that the code is readable without comments...
If you are not doing one of those two things, how do you make sure that the comments get updated?
I feel like I am repeating myself here, but as hard as you try, comments fall victim to entropy much quicker than the actual code. You can try to be diligent, but without a tool to help you, comment entropy is inevitable. Unless, of course, you are a one-man shop?
regardless of how you handle the code review, whether it be pair program or a full blown code review w/ multiple peeps, wouldn't it be easier to have the person in question write a better comment than to have them rewrite all the code so that that group of people can read it? what happens when you bring in a new team that needs to read the code, but the code isn't readable to them?
I suppose if you don't accept my premise that applying these techniques will make your code as readable as if it had comments, then I can't convince you that it can be done.
I can say that the developers on my team also agree with this approach (the sentiment has evolved over time). I have never heard of a complaint that the code needs more comments... only that the comments that are there are useless or wrong. They all agree that our code is better for writing code this way.
So, I guess I am trying to say that what I am proposing in my post CAN be done, regardless if you are able to imagine it.
Oh, I'm not saying it can't be done; it can, obviously :) what i'm saying is that it doesn't mean that b/c it's readable to you that it will be readable to someone else down the road. for instance, what if you moved on to another company, and someone else was hired in to maintain your code? they may not be able to follow the code that you thought was readable. thus, if they had a short comment about said code, the chance they'll be able to understand what you're trying to do increases dramatically. writing code so that only a certain amount of people understand it is never a good idea. it's always best to make sure it can be understood easily.
My point is that if you can come up with a useful comment, you can almost always write that comment in code instead. Anything in addition to that is usually repeating yourself (DRY). If said comment is too long to write in code, then write a comment (as my blog post does concede), but make sure it is a "Why" comment (not a "How" comment) and don't drag on too long or else the comment will fall victim to entropy. (It probably will anyways)
I am growing tired of this argument... not because I don't respect your thoughts... but because it feels like we are beating a dead horse. We are not likely to change the other's mind. This will be my last response on this topic. If you would like the last word, it is yours...
I agree with u on the part about comments getting too long. I'm not trying to say everything should be commented, but there should be enough that you can figure it out without having to pour thru the code. Good discussion!
Post a Comment