This question exists because it has historical significance, but it is not considered a good, on-topic question for this site, so please do not use it as evidence that you can ask similar questions here. This question and its answers are frozen and cannot be changed. More info: help center.

As it currently stands, this question is not a good fit for our Q&A format. We expect answers to be supported by facts, references, or expertise, but this question will likely solicit debate, arguments, polling, or extended discussion. If you feel that this question can be improved and possibly reopened, visit the help center for guidance.
If this question can be reworded to fit the rules in the help center, please edit the question.

Your coworker made a sweeping generalization, which automatically means he is wrong. :)
–
Alex FeinmanOct 1 '10 at 17:25

5

@Mongus, I disagree. The comments in your example is bad not because they are comments, but because they are TOO close to the code which then changes. They should say WHY and not WHAT.
–
user1249Dec 27 '10 at 13:07

5

@Alex, isn't that a sweeping generalization, which is therefore wrong (resulting in him not being wrong anyway)?
–
user1249Jan 31 '11 at 10:25

35 Answers
35

If I wanted to know what was happening in a method or block, I would read the code. I would hope, anyway, that any developers working on a given project were at least familiar enough with the development language to read what is written and understand what it is doing.

In some cases of extreme optimization, you might be using techniques that makes it difficult for someone to follow what your code is doing. In these cases, comments can and should be used to not only explain why you have such optimizations, but what the code is doing. A good rule of thumb would be to have someone else (or multiple other people) familiar with the implementation language and project look at your code - if they can't understand both the why and the how, then you should comment both the why and the how.

However, what's not clear in the code is why you have done something. If you take an approach that might not be obvious to others, you should have a comment that explains why you made the decisions that you did. I would suspect that you might not even realize that a comment is needed until after something like a code review, where people want to know why you did X instead of Y - you can capture your answer in the code for everyone else who looks at it in the future.

The most important thing, though, is to change your comments when you change your code. If you change an algorithm, be sure to update the comments with why you went with algorithm X over Y. Stale comments are an even bigger code smell.

@back2dos If you are saying that you can't look at most code and understand what it is doing, then you need to work on that. Except for some highly optimized algorithms that exploit tricks of a given language, I have never seen code written in a language that I know that I can't understand the "what" of what is happening.
–
Thomas Owens♦Sep 12 '10 at 22:24

23

I'm not alone to think reading code is harder than writing it. And I definitely believe, that reading code is harder, than writing comments. If I were to maintain your code, then if there were comments in it, I could remove them automatically, in case I felt so darn cool and I'd be so bored that I'd have my employer pay me for trying to understand code, that is sensibly commented. If there are no comments, I'm left with pretty much no choice. I think, this is just a little selfish towards the poor soul, who's gonna have to maintain your code.
–
back2dosSep 13 '10 at 7:21

8

I agree with this answer vis a vis comments, but I've also seen it cited as an excuse for lack of documentation, which is wrong. Reading code is a pain in the ass sometimes. You shouldn't have to look at the code for a method to figure out what that method does. You should be able to figure it out from the name and get some more details from the docs. When reading code, you often have to switch from class to class, and file to file. This especially a problem in dynamic languages, where writing an IDE that can handle all this is non-trivial.
–
notJimSep 14 '10 at 1:48

3

"Only if the comment describes what the code is doing." Or if the comment describes what the code used to do; the code has changed but the comment hasn't.
–
Bruce AldermanNov 11 '10 at 22:29

This is particularly irritating to hear at the moment, I spent some time this weekend looking at very well-named, very clean, uncommented code implementing a research algorithm (one that isn't actually published). I'm high-level familiar with it, the guy sitting next to me was the inventor, and the code was written a few years ago by someone else. We could barely follow it.

I'm curious to look at "well-named, very clean, uncommented code" that is hard to follow. Any code that I would classify as such has been very easy to follow. I certainly wouldn't go as far as "Your co-worker is not experienced enough, obviously".
–
LiggySep 15 '10 at 1:34

8

@Liggy: I would. It's a research algorithm, not a line-of-business application.
–
Paul NathanSep 15 '10 at 17:14

9

I once had a piece of code where you had to populate fields in a database column object (3rd party) in the "wrong" order (defined by the "logic" of the process and our coding standards) - do it in the order we'd normally use and it would crash. No amount of reading the code could tell you this so it positively, absolutely, had to have a comment - and it wasn't a smell, at least not in code over which we had control (which is the bottom line). A complete and utter lack of comments is as much a smell as poor comments.
–
MurphSep 27 '10 at 7:28

I agree with the why not how part, and your refactoring works in this example, but with more complex code, no amount of variable/function renaming/refactoring is gonna have it make perfect sense, and comments will still be needed there.
–
GStoSep 9 '10 at 2:09

3

Good example, but why is the system working with cents as opposed to dollars? That question becomes relevant in financial systems where you might see fractional currency come into play.
–
rjziiDec 15 '10 at 14:07

3

@Stuart the name of the function should say what it does.
–
AlbJan 31 '11 at 12:19

3

@GSto, I couldn't disagree more. If the code is complex, it should be broken into smaller methods/functions with appropriate names that describe the action.
–
CaffGeekFeb 4 '11 at 5:49

1

You assume, that you have complete control over the code base.
–
LennyProgrammersFeb 8 '11 at 14:13

I declare your co-worker a heretic! Where are my heretic-burnin' boots?

Obsessive commenting is bad and a maintenance headache, and comments are no replacement for well-named methods, classes, variables, etc. But sometimes putting why something is the way it is can be immensely valuable for the poor idiot who has to maintain the code in six months -- particularly when that poor idiot winds up being you.

Some actual comments from the code I'm working on:

// If this happens, somebody's been screwing around with the database definitions and
// has removed the restriction that a given alarm may have only one entry in the
// notifications table. Bad maintenance programmer! Bad! No biscuit!
// If an alert is active on our side but inactive on theirs, that might mean
// they closed the alert. (Or that we just haven't told them about it yet.) The
// logic comes later; for now, we'll just compile it in a list.
// If we know for a fact that an alarm isn't getting through, we're going to whine pretty
// aggressively about it until it gets fixed.

Don't forget that you have to maintain and keep in sync also comments... outdated or wrong comments can be a terrible pain! And, as a general rule, commenting too much can be a symptom of bad programming.

Good naming convention and well structured code will help you decrease the comments need. Don`t forget that each line of comments you add it's a new line to maintain!!
–
Gabriel MongeonSep 1 '10 at 19:47

81

I hate when people use that second example in your question. } //end while just means the beginning of the while loop is so far away, you can't even see it, and there are no hints that the code you're looking at is in a loop. Heavy refactoring should be seriously preferred to comments about how the code is structured.
–
Carson MyersSep 1 '10 at 20:03

4

@Carson: while keeping blocks short is a well known rule, it doesn't mean that we always can apply it.
–
LorenzoSep 1 '10 at 20:15

4

@Carson: One of the projects I work on has insufficient code review, which lead to a collection of JSPs with a cyclomatic complexity of "OMFG JUST KILL YOURSELF." Refactoring the bloody things represents days of work that need to be spent elsewhere. Those } // end while comments can be a lifesaver.
–
BlairHippoSep 2 '10 at 15:26

11

@BlairHippo: "[A] code smell is any symptom in the source code of a program that possibly indicates a deeper problem". Of course the comment is a life saver, but the OMGWTF-loop is the aforementioned "deeper problem" and the necessity of the life saver is a clear indicator ;)
–
back2dosSep 12 '10 at 18:10

Categorically defining a method or process as a "code smell" is a "zealotry smell". The term is becoming the new "considered harmful".

Please remember that all of these sort of things are supposed to be guidelines.

Many of the other answers give good advice as to when comments are warranted.

Personally I use very few comments. Explain the purpose of non-obvious processes and leave the occasional death-threat to anyone that might be considering altering things on their own that required weeks of tuning.

Refactoring everything until a kindergartner can understand it is likely not an efficient use of your time, and probably will not perform as well as a more concise version.

Comments don't affect run time,so the only negative issue to consider is the maintenance.

I thought "anti-pattern" was the new "considered harmful". A code smell is just something that needs a closer review for possible cleanup.
–
Jeffrey HantinOct 7 '10 at 20:58

1

I don't disagree that anti-pattern also qualifies. They both get used that way with anti-pattern being used instead of smell when the design is complex enough that it is obviously a deliberate choice. In either case I disagree with the concept that there is an absolute source of correct on these things.
–
BillOct 8 '10 at 1:06

4

+1 for "Refactoring everything until a kindergartner can understand it is likely not an efficient use of your time, and probably will not perform as well as a more concise version."
–
EarlzFeb 19 '11 at 5:00

// DO NOT TOUCH THE FOLLOWING TWO LINES; the ExtJS UploadForm requires it exactly like that!!!
response.contentType="text/html" // must be text/html so the browser renders the response within the invisible iframe, where ExtJS can access it
render '{"success":true}' // ExtJS expects that, otherwise it will call the failure handler instead of the succss handler

Note that a CodeSmell is a hint that something might be wrong, not a certainty. A perfectly good idiom may be considered a CodeSmell because it's often misused, or because there's a simpler alternative that works in most cases. Calling something a CodeSmell is not an attack; it's simply a sign that a closer look is warranted.

So what does it mean that comments are a code-smell: it means that when you see a comment, you should pause and think: "Hmmm, I sense a hint that something could be improved". Perhaps you can rename a variable, perform the "extract method"-refactoring -- or perhaps the comment is actually the best solution.

That is what it means for comments to be code smells.

EDIT:
I just stumpled upon these two articles, which explains it better than me:

I'm stunned that took 2 months for this answer to come up. It demonstrates how widespread the misunderstanding of the term is.
–
Paul ButcherFeb 10 '11 at 10:17

1

@Stuart: You're looking at two chunks of code, both with appropriate levels of comments. (This issue is not about the appropriate number of comments, it's about how you judge code based on the number of comments.) One has no comments, the other has tons. Which do you look more closely at? -- Comments are a sign that code is complicated and subtle and thus more likely to have issues.
–
David SchwartzSep 7 '11 at 13:59

I think the rule is quite simple: imagine a complete stranger seeing your code. You probably will be a stranger to your own code in 5 years.
Try to minimize the mental effort to understand your code for this stranger.

+1 To any dev who hasn't been working on a single project long enough to experience this, believe me it will happen. Anytime I learn this the hard way and have to relearn my own code, I don't let myself make the same mistake twice and comment it before I move on to something else.
–
NickCSep 17 '10 at 20:33

4

I regularily become a psychopath when i see unreadable code.
–
LennyProgrammersSep 27 '10 at 12:32

A good idea to have the right comments is to start with writing comments.

// This function will do something with the parameters,
// the parameters should be good according to some rules.
myFunction(parameters)
{
// It will do some things to get started.
// It will do more with the stuff.
// It will end doing things with the stuff.
}

This allows you to extract methods easily to even get rid of the comments,just let the code tell these things! See how this is rewritten (cut/paste) in a very nice way:

// This function will do something with the parameters,
// the parameters should be good according to some rules.
myfunction(parameters)
{
var someThing = initializedWithSomething;
doSomethingWith(someThing);
doMoreWith(someThing);
endDoingThingsWith(someThing);
return someThing;
}
// This function will do some things to get started,
// the parameters should be good according to some rules.
doSomethingWith(parameters)
{
parameters.manipulateInSomeWay();
... etc ...
}
... etc ...

For things that can't be separated just don't extract methods and type the code under the comments.

This is what I see as an useful way to keep commenting to a minimum, it's really useless to go commenting each line... Only document a single line if it's about a magic value initialization or where it makes sense.

If parameters are used too much, then they should be private members in your class.

I think the answer is the usual "It depends" one. Commenting code just to comment code is a smell. Commenting code because you're using an obscure algorithm that's an order of magnitude faster saves the maintenance programmer (usually me 6 months after I wrote it) half a day of poking through the code to determine what it's doing.

Code comments are definitely not a "code smell". This belief typically comes from the fact that comments can become stale (out of date) and can be difficult to maintain. However, having good comments which explain why the code is doing something a certain way can (and usually is) important for maintenance.

Good comments make it easier to
understand what the code is doing and,
more important, why it is doing it a
particular way. Comments are meant to
be read by programmers and should be
clear and precise. A comment that is
hard to understand or incorrect isn’t
much better than having had no comment
at all.

Adding clear and precise comments to
your code means that you don’t have to
rely on memory to understand the
“what” and “why” of a section of code.
This is most important when you look
at that code later on, or someone else
must look at your code. Because
comments become part of the textual
content of your code, they should
follow good writing principles in
addition to being clearly written.

To
write a good comment, you should do
your best to document the purpose of
the code (the why, not how) and
indicate the reasoning and logic
behind the code as clearly as
possible. Ideally, comments should be
written at the same time as you write
the code. If you wait, you probably
won’t go back and add them.

Comments can become stale, but that's true for everything that's not verified by a compiler or a unit test, like the meaning of class, function and variable names.
–
LennyProgrammersSep 10 '10 at 8:39

2

I agree. My point is, there is a risk of it becoming stale, yes. But when i have a function doBar() and after 3 years it doesn't "do bar", but it "does bar and foo except on wednesdays" then the meaning of functions can get stale too. And variable names. But i hope noone takes this for a reason to not give functions and variables meaningful names.
–
LennyProgrammersSep 10 '10 at 15:25

If the code has been written in a particular way to avoid a problem present in a library (both a third-party library, or a library that comes with the compiler), then it makes sense to comment it.
It also make sense to comment code that needs to be changed in future versions, or when using a new version of a library, or when passing from PHP4 to PHP5, for example.

Even the most well-written book still likely has an introduction and chapter titles. Comments in well-documented code are still useful to describe high-level concepts, and explain how the code is organized.

It's my impression that sometimes FLOSS license preamples are frequently used in lieu of file documentation. The GPL/BSDL makes a nice filler text, and after that you seldomly see any other comment block.

I disagree with the idea that writing comments to explain the code are bad. This completely ignores the fact that code has bugs. It might be clear what the code does without comments. It's less likely to be clear what the code is supposed to do. Without comments how do you know if the results are wrong, or they're being used incorrectly?

The comments should explain the intent of the code, so that if there is a mistake, someone reading the comments+code has a chance of finding it.

I generally find myself writing inline comments before I write the code. This way it's clear what I'm trying to write code to do, and reduces getting lost in an algorithm without really knowing what you're trying to do.

Unit tests help a lot to determine if the results are wrong. If you read some code and think it does X when in reality it does Y, then it's possible the code isn't written in a readable enough way. I'm not sure what you mean about the results being used incorrectly. A comment will not protect you against someone consuming your code in strange ways.
–
Anna Lear♦Feb 27 '11 at 15:36

Comments that are put in because someone thinks it's ok to have 700 lines in one method are a smell.

Comments that are there because you know if you don't put in a comment, someone will make the same mistake yet again are a smell.

Comments put in because some code analysis tool demands it are also a smell.

People that won't ever put in a comment, or write even a little help for other developers are also a smell. I'm amazed at how many people won't write stuff down, but will then turn around and acknowledge they can't remember what they did 3 months ago. I don't like to write docs, but I like to have to tell people the same thing over and over again even less.

@Ian: The program is an IBM-PC bootloader. You can't write it in anything other than assembly, because you need total control of exactly where the program is located in RAM, where the last short appears, and some of the hardware interrupts. Seriously, get yourself a copy of NASM. Assemble it, write it to the boot sector of a floppy or USB stick, and boot it. Though you'll probably find it doesn't work as expected because of the bug. Now find the bug... Regardless, I'm sure that in 20 years from now, people will ask the same thing of uncommented Java.
–
AntFeb 8 '11 at 15:02

1

When coding assembly, I'd do what everyone else does - comment every single line with what it's doing. The line in question would have had the comment "check if letter is 0", as the code uses C-style 0-terminated strings. With that comment in place, it's easy to see that the code is doing a cmp with 1, which means it either gets stuck in an infinite loop or prints garbage until it hits a random 1 in RAM. I might also have added a comment about the fact that the strings were 0-terminated, which isn't apparent from the code. Is there such a thing as automated unit testing for assembly coding?
–
AntMay 20 '11 at 11:47

I think code commenting gets a very poor start to life. I don't know about these days, but when I was first being taught programming at school I got assignments of the nature of "Write a program that prints the numbers one to ten on separate lines. Make sure you comment your code." You'd get marked down if you didn't add comments because commenting your code is a GOOD THING.

But what is there to say about such a trivial process? So you end up writing the classic

i++; // add one to the "i" counter.

just to get a decent grade and, if you've any nous at all, instantly forming a very low opinion of code comments.

Code commenting is not a GOOD THING. It's a SOMETIMES NECESSARY THING, and Thomas Owens in the top answer provides an excellent explanation of the situations in which it's necessary. However, these situations rarely crop up in homework-type assignments.

In many ways, adding a comment should be considered a last-resort choice, when what needs to be said cannot be said clearly in the active parts of the programming language. Although object naming can go stale, various human and computer lack-of-feedback mechanisms make it easy to forget to maintain comments and consequently comments go stale much more quickly than active code. For that reason, where a choice is possible, changing code to make it clearer should always be preferred to annotating unclear code with comments.

You have to keep a balance between code and comments... Usually I try to add some comments that resume a block of code. Not because I won't be able to understand the code (well, that also), but because I can read faster my own code and locate specific sections where the important stuff it's happening.

Anyway, my own personal criteria is "when in doubt, comment". I prefer to have a redundant line than a completely cryptic line that I'm not going to be able to understand. I can always remove comments on a code review, after a while (and I usually do)

Also, comments are quite helpful adding "caveats" like "Be careful! If the format of the input is not ASCII, this code will have to change!"

Or, alternatively, the comment may reflect the fact that the code is addressing a complex algorithm where the code becomes inherently not obvious or because the code is doing something odd due to factors beyond your control (e.g. interacting with an external service).
–
MurphSep 27 '10 at 7:37

Of course comments are a code smell...

every programmer knows we all eventually turn insane due to the amount of work, debugging, or just plain madness we run into.

"Do this!" your project manager says.

You respond, "It can't be done."

They say, "Then we will find someone else to do it."

You say, "OK, well maybe it can be done."

And then spend the next X number of days.. weeks.. months.. trying to figure it out. Throughout the process, you will try and fail, and try and fail. We all do this. The true answer is there are two types of programmers, those that comment and those that don't.

1) Those that do are either making their own job easier by documenting for future reference, commenting out failed routines that didn't work (the smell is not deleting them after finding the one that works.), or breaking up the code with comment formatting to hopefully make it a bit easier to read or understand. Seriously, I can't blame them. But in the end, they snap and then you have this:
// dammit this code sucks! swear! curse! i hate it! i am going to write something here to vent my anger!!!!

2) Those that don't are either pretending to be a superhero, or are living in a cave. They simply have reckless disregard for others, themselves, and could care less about the code, or what meaning it could possibly have for later.

Now don't get me wrong.. self-documenting variables and functions can avoid this entirely.. and trust me you can never do enough code-cleanup. But the simple truth is that as long as you keep backups, you can ALWAYS delete comments.

In response to 1. The true smell in commented routines is not deleting them right away when you decide they may be dead ends and want to try something different---this is because they should be available in version control if you decide to revisit them.
–
SharpieJan 10 '11 at 0:12

I'd argue that not using some comments in your code is a code smell. While I agree that code should be self documenting as much as possible, you hit a certain point where you are going to see code that makes no sense regardless of how well the code is written. I've seen some code in business applications where the comments are pretty much mandatory because:

You need to do something on a case by case basis and there's no good logic for it.

The code will likely change in a year or two when the laws are changed and you want to find it again quickly.

Someone edited the code in the past because they didn't understand what the code was doing.

Also, company style guides might tell you to do something a certain way - if they say that your could should have comments outlining what blocks of code in a function is doing, then include the comments.

There's a big fundamental difference between comments and code: comments are a way for people to communicate ideas to other people, whereas code is primarily meant for the computer. There are many aspects in "code" that's also only for humans, like naming and indentation. But comments are written strictly for humans, by humans.

Therefore, writing comments is every bit as difficult as any written human communication! The writer should have a clear conception of who the audience is, and what kind of text they will need. How can you know who will read your comments in ten, twenty years? What if the person is from a completely different culture? Etc. I hope everybody understands this.

Even inside the small homogeneous culture I live in, it's just so difficult to communicate ideas to other people. Human communication usually fails, except by accident.