DEV Community

loading...

Discussion on: Comments Are The Only "Code Smell"

Collapse
valeriavg profile image
Valeria

I disagree. Comment itself is not a "code smell", its misuse is.

There are situations where leaving a comment is a necessity.
Comments are not meant to describe things, but to clarify certain statements and leave notes, regarding use of certain constructions and tools.

For example :

// typeof null is 'object'
function getType(value){
  if(value === null) return 'null';
  return typeof value;
}

// Limit degrees to [0,360), 360 degrees is 0
const degrees = userInput & 360;

// Due to a bug in 3rd party module we need to perform this check
// [link]
const result = thirdPartyModule.exec(validateValue(value));
Collapse
bytebodger profile image
Adam Nathaniel Davis Author

I'm pretty sure that this is exactly what I described in the article.

Collapse
valeriavg profile image
Valeria • Edited

Please forgive me if I misinterpreted your article.
It seemed to me, that your article main idea was "don't use comments unless you're absolutely sure", opposite to "use comments code wisely".

Collapse
merri profile image
Vesa Piittinen

I disagree with the above samples. I hope I don't go too much into detail :)

On first one I'd remove the comment and write a test which would ensure correct output. If later on somebody gets an idea to remove the value === null because it is "not needed" the test will break and they'll see why.

Second one should be userInput % 360. To me the comment tells what the code does (modulo) so there is no need to reiterate it, and I would remove the comment upon seeing it. It also lies because the code doesn't do anything to check against negative values, so the comment could give a false feeling of correctness. Tests would ensure correctness.

For the third one I'd add emphasis that it should tell exactly which 3rd party module is in question, and also suggest a short comment on why the bug has to be worked around the way it is (especially if the code for it isn't obvious). Reason: I've seen people use these "due to bug in 3rd party..." comments as-is and they're not useful. Of course the link is the meat, but because the Internet cannot guarantee links to work forever I'd always also add the minimum necessary required for understanding into the comment.

Collapse
valeriavg profile image
Valeria

I agree, tests will prevent things from being removed without notice, but having one line right by the function would (hopefully) prevent the "refactoring" altogether.

Good point on modulus, bad example.
It was implied that the comment would state the name of the package or in the worst case link to GitHub issue will.

Bottom line, If you're not sure if you should leave the comment - you should.
It's a text, it's a core feature of every single language and it's meant to be used, not feared.

Thread Thread
merri profile image
Vesa Piittinen • Edited

Maybe I've had bad experiences or bad luck, but I've noticed comments to be rather ineffective against "refactoring". You need only one person with too strong an opinion and less experience who successfully ignores what the comment warns or informs about, and goes ahead changing and breaking things.

Code reviews don't perfectly protect against this as those may get through by another who doesn't stop to care about the comment, possibly because it doesn't appear in the diff. Only tests seem to successfully protect against this, at least against breaking stuff. You'd have to go for slightly malicious mindset to start removing or changing valid tests. These "refactors" are often done thinking it is an improvement, with no malicious intent.

This is one reason why I keep comments as the absolute last tool to convey understanding that the code or naming things can't provide.