Thursday, 10 February 2011

Code Smells are signs, not problems

The concept of Code Smells seems to be somewhat misunderstood.

In particular, the claim that "Comments are a Code Smell" seems to be an extremely dangerous one to make. If you espouse this position, you are liable to be called unprofessional, or an idiot.

Even el reg seems to have gotten the wrong end of the stick, and claims that to declare comments to be a smell is to "spoil a good guideline by taking it to extremes".

It's not a smell!!!!1!!11one1!


Having questioned your abilities (and possibly the virtues of your mother), some responders go on to angrily declare that the only problem is in comments that describe what the code does, and that comments should state why, not what.

Others may assert that sometimes you need commentary to explain a counterintuitive approach forced upon you by an underlying API.

Others still, may claim that they have been struggling with some tangled mess of illegible code, that could have seriously benefited from commentary.

This post on programmers.stackexchange.com, engendered some responses of the kind I describe.

The problem is that these responses never argue against the assertion that "comments are a code smell", but against the assertion (which nobody has made), that comments are an antipattern.

The common feature of those who argue against the assertion that "Comments are a Code Smell", is that (as in this question) they seem to interpret "Code Smell" as a synonym for "Antipattern". Often, they then go on to provide proof that comments are, indeed, a code smell.

What is a code smell?


Here are some definitions of a code smell:

Wikipedia:
"In computer programming, code smell is any symptom in the source code of a program that possibly indicates a deeper problem."
WardsWiki (where the term was first coined):
"A code smell is a hint that something has gone wrong somewhere in your code. ... Note that a CodeSmell is a hint that something might be wrong, not a certainty."

Pay attention to the emphasised words in the WardsWiki quote. Basically, code smells are symptoms, not diseases. If I have a runny nose, it could be because I have a cold, or I could have just been surfing, and the nasal enema that comes with a wipeout is working its magic. Any number of things could be the cause, but not all of them require treatment.

Rebuttals rebutted

Considering smells as hints, let's revisit the "It's not a smell!!" responses from above, and see how each instance proves that comments are, indeed, a smell:


Specific Smell:

There are comments describing what the code does.

Implies:

The author of the comments has a poor understanding of the programming language.

The deeper problem:

One of your programmers is incompetent

What may be wrong:

The incompetent programmer has probably introduced diverse bugs.

Why it might not be wrong:

The code might have been written as part of a course - either by a student who needs to demonstrate they can read code, or by an instructor to describe the code to some neophytes.


Specific Smell:

There are comments explaining why some code does what it does.

Implies:

Your motives are non-obvious.

The deeper problem:

Your motives are arcane and unnecessary.

What may be wrong:

Your incorrect motives have led to some incorrect or unnecessary code.

Why it might not be wrong:

The code is necessarily complex and confusing - complicated mathematics (for example) can be difficult to explain without additional commentary. There may be an accepted domain-specific convention that is being followed, that renders the code obvious to a Subject Matter Expert, but would appear counterintuitive to an experienced software developer from a different subject.


Specific Smell:

You have to comment around the foibles of a 3rd party API

Implies:

The API is counterintuitive

The deeper problem:

Counterintuitive APIs make for a potentially fragile system.

What may be wrong:

Somewhere in the code, the API's conventions may not have been correctly followed.

Why it might not be wrong:

The counterintuitive API may be the best, or only, fit for the job.


Specific Smell:

Some uncommented code was hard to understand.

Implies:

The code needs comments to be understandable.

The deeper problem:

Code that can't easily be understood without comments is more likely to suffer a disconnect between code and comments, or be just plain wrong.

What may be wrong:

Comments may not reflect the code, developers will be reluctant to touch it, cargo-cult programming may arise. Code that is hard to understand is easy to break and scary to change; regardless of how much commentary it contains.

Also, even a developer who diligently updates the comments whenever he updates the code, may misunderstand the side-effects of a change (or quite likely, trust an incorrect comment elsewhere), and update comments incorrectly.

Why it might not be wrong:

If you are programming in Assembly or Brainfuck, the only scope you have for increasing the readability is by commenting.

Note that if you are adding commentary to some spaghetti code to make sense of it before refactoring, then that is still something wrong.


Anyone who still asserts that comments are not a code smell, please give me some examples of non-smelly comments, and I'll see if I can add them to the list.