Documentation – Can Commented-Out Code Be Valuable?

commentsdocumentation

I wrote the following code:

if (boutique == null) {
    boutique = new Boutique();

    boutique.setSite(site);
    boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
    boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
    boutique.setNom(fluxBoutique.getNom());
    boutique.setSelected(false);
    boutique.setIdWebSC(fluxBoutique.getId());
    boutique.setDateModification(new Date());

    boutiqueDao.persist(boutique);
} else {
    boutique.setSite(site);
    boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
    boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
    boutique.setNom(fluxBoutique.getNom());
    //boutique.setSelected(false);
    boutique.setIdWebSC(fluxBoutique.getId());
    boutique.setDateModification(new Date());

    boutiqueDao.merge(boutique);
}

There is a commented-out line here. But I think it makes the code clearer, by making obvious what the difference is between if and else. The difference is even more noticeable with color highlighting .

Can commenting out code like this ever be a good idea?

Best Answer

Most of the answers focus on how to refactor this one specific case, but let me offer a general answer to why commented out code is usually bad:

First, commented out code isn't compiled. This is obvious, but it means that:

The code might not even work.
When the comment's dependencies change it will not obviously break.

Commented code is very much "dead code". The longer it sits there, the more it rots and provides less and less value to the next developer.

Second, the purpose is unclear. You really need a longer comment that provides context for why there are randomly commented lines. When I see just a commented line of code, I have to research how it got there just to understand why it got there. Who wrote it? What commit? What was the commit message/context? Etcetera.

Consider alternatives:

If the goal is the provide examples of using a function/api, then provide a unit test. Unit tests are real code, and will break when they are no longer correct.
If the purpose is to preserve a previous version of the code, use source control. I'd much rather checkout a previous version then toggle comments throughout the codebase to "revert" a change.
If the purpose is to maintain an alternate version of the same code, use source control (again). That is what branches are for, after all.
If the purpose is to clarify structure, consider how you can restructure the code to make it more obvious. Most of the other answers are good examples of how you might do this.

Related Solutions

Java Documentation – Is It Necessary to Write Javadoc Comments for Every Parameter?

Javadoc (and, in the Microsoft word, XMLDoc) annotations are not comments, they are documentation.

Comments can be as sparse as you want them to be; assuming your code is halfway readable, then ordinary comments are merely signposts to aid future developers in understanding/maintaining the code that they've already been staring at for two hours.

The documentation represents a contract between a unit of code and its callers. It is part of the public API. Always assume that Javadoc/XMLdoc will end up either in a help file or in an autocomplete/intellisense/code-completion popup, and be observed by people who are not examining the internals of your code but merely wish to use it for some purpose of their own.

Argument/parameter names are never self-explanatory. You always think they are when you've spent the past day working on the code, but try coming back to it after a 2-week vacation and you'll see just how unhelpful they really are.

Don't misunderstand me - it's important to choose meaningful names for variables and arguments. But that is a code concern, not a documentation concern. Don't take the phrase "self-documenting" too literally; that is meant in the context of internal documentation (comments), not external documentation (contracts).

Clean Code – Comments vs Class Documentation

As others have said, there's a difference between API-documenting comments and in-line comments. From my perspective, the main difference is that an in-line comment is read alongside the code, whereas a documentation comment is read alongside the signature of whatever you're commenting.

Given this, we can apply the same DRY principle. Is the comment saying the same thing as the signature? Let's look at your example:

Retrieves a product by its id

This part just repeats what we already see from the name GetById plus the return type Product. It also raises the question what the difference between "getting" and "retrieving" is, and what bearing code vs. comment has on that distinction. So it's needless and slightly confusing. If anything, it's getting in the way of the actually useful, second part of the comment:

returns null if no product was found.

Ah! That's something we definitely can't know for sure just from the signature, and provides useful information.

Now take this a step further. When people talk about comments as code smells, the question isn't whether the code as it is needs a comment, but whether the comment indicates that the code could be written better, to express the information in the comment. That's what "code smell" means- it doesn't mean "don't do this!", it means "if you're doing this, it could be a sign there's a problem".

So if your colleagues tell you this comment about null is a code smell, you should simply ask them: "Okay, how should I express this then?" If they have a feasible answer, you've learned something. If not, it'll probably kill their complaints dead.

Regarding this specific case, generally the null issue is well known to be a difficult one. There's a reason code bases are littered with guard clauses, why null checks are a popular precondition for code contracts, why the existence of null has been called a "billion-dollar mistake". There aren't that many viable options. One popular one, though, found in C# is the Try... convention:

public bool TryGetById(int productId, out Product product);

In other languages, it may be idiomatic to use a type (often called something like Optional or Maybe) to indicate a result that may or may not be there:

public Optional<Product> GetById(int productId);

So in a way, this anti-comment stance has gotten us somewhere: we've at least thought about whether this comment represents a smell, and what alternatives might exist for us.

Whether we should actually prefer these over the original signature is a whole other debate, but we at least have options for expressing through code rather than comments what happens when no product is found. You should discuss with your colleagues which of these options they think is better and why, and hopefully help move on beyond blanket dogmatic statements about comments.

Best Answer

Related Solutions

Java Documentation – Is It Necessary to Write Javadoc Comments for Every Parameter?

Clean Code – Comments vs Class Documentation

Related Topic