Clean Code – Comments vs Class Documentation

clean codecomments

I'm having some discussions with my new colleagues regarding commenting. We both like Clean Code, and I'm perfectly fine with the fact that inline code comments should be avoided and that class and methods names should be used to express what they do.

However, I'm a big fan of adding small class summaries that tries to explain the purpose of the class and what is actually represents, primarily so that its easy to maintain the single responsibility principle pattern. I'm also used to adding one-line summaries to methods that explains what the method is supposed to do. A typical example is the simple method

public Product GetById(int productId) {...}

I'm adding the following method summary

/// <summary>
/// Retrieves a product by its id, returns null if no product was found.
/// </summary

I believe that the fact that the method returns null should be documented. A developer that wants to call a method should not have to open up my code in order to see if the method returns null or throws an exception. Sometimes it's part of an interface, so the developer doesn't even know which underlying code is running?

However, my colleagues think that these kinds of comments are "code smell" and that "comments are always failures" (Robert C. Martin).

Is there a way to express and communicate these types of knowledge without adding comments? Since I'm a big fan of Robert C. Martin, I'm getting a bit confused. Are summaries the same as comments and therefore always failures?

This is not a question about in-line comments.

Best Answer

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.

Related Solutions

Documentation – Are XML Comments Necessary?

If your comments only look like this:

/// <summary>
/// Gets or sets the sublot number.
/// </summary>
/// <value>
/// The sublot number.
/// </value>

Then yes, they are not all that useful. If they read something like this:

/// <summary>
/// Gets or sets the sublot number.
/// Note that the sublot number is only used by the legacy inventory system.
/// Latest version of the online inventory system does not use this, so you can leave it null. 
/// Some vendors require it but if you don't set it they'll send a request for it specifically.
/// </summary>
/// <value>
/// The sublot number.
/// </value>

Then I'd say they have value. So to answer your question: Comments are necessary when they say something that the code does not say.

An exception: it is good to have comments on anything that is publicly accessible if you're writing a library/API that will be available to the public. I hate using a library and seeing a function named getAPCDGFSocket() with no explanation of what an APCDGFSocket is (I'd be happy with something as simple as This gets the Async Process Coordinator Data Generator File Socket). So in that case, I'd say use some tool to generate all comments and then manually tweak the ones that need it (and please make sure your cryptic acronyms are explained).

Also, getters/setters are generally bad examples for "are comments necessary?" because they are usually quite obvious and comments aren't necessary. Comments are more important on functions that perform some algorithm where some explanation of why things are being done they way they are could make the code much more understandable and also make it easier for future programmers to work with.

...and finally, I'm pretty sure that this question is relevant for all styles of comments, not just those that are formatted using XML (which you are using because you're working in a .NET environment).

Commenting Strategies in C# and ASP.NET Web API

There is no definitive answer: some people will be inclined to add comments to every class and method; others will try to avoid commenting things which are already explicit enough.

If you follow StyleCop default rules, you have to comment every member, including private fields. You might want to suppress the warnings if the code is really repetitive. But if it is, you may ask yourself why is it not automatically generated (note: automatically generated code is marked specifically, and is not processed by StyleCop).
If you don't, feel free to comment as much or little as you feel appropriate. If I were maintaining the piece of code from your question, I would have added comments on every method, just to simplify my work later: I'd rather read a sentence in a tooltip in Visual Studio rather than try to guess right the meaning of the method. On the other hand, I would avoid commenting something like Title property of the class Product.

In all cases, try to bring something useful on those comments, not just repeat the name of the method or property. For example, if the class Product has a property Description, one way is to write:

/// <summary>
/// Gets the description of the product.
/// </summary>

This comment doesn't bring anything useful. Another possibility is this one:

/// <summary>
/// Gets the piece of text which is displayed publicly and is usually written by the
/// managers. The description is localized for the current user.
/// </summary>

Here, you bring three pieces of useful information:

The description is public. The web app may also have an internal summary field where managers can write text for themselves.
The description is usually written by managers. This helps understanding the business logic.
The description is a string translated in the language of the current user (probably CultureInfo.CurrentCulture).

Best Answer

Related Solutions

Documentation – Are XML Comments Necessary?

Commenting Strategies in C# and ASP.NET Web API

Related Topic