There was some code where there were 2 statements that look similar in meaning:

• Details is null
• No Details

  if (Details == null)
     return (bool?)null;
 
  if (!Details.Any())
     continue;

Yet, both those statements returned a different value. The first returns null, the second continues iterating the loop that the code was in and could return a completely different value depending on what was next in the list.          

So a reviewing developer writes a comment on the review:

“It is not clear why one should return and the other continues here. Maybe a quick inline comment to explain?”

(So the reviewing developer wanted them to write a code comment alongside the code, so anyone reading the code would be able to read the code comment explaining what the code means – therefore quickly understand the intent.)

The authoring developer responds:

“Explaining here as adding comments will open up code smells,

The Message can contain more than one ‘Detail’ and it will validate the following cases….”

(It’s not important for this blog to post his full explanation. The key thing is his refusal to add the code comment because code comments are apparently a “code smell”. The reviewing developer counters this statement accurately…)

Reviewing Developer:

Code smells are code that cause people headaches to read and understand, not what Static Analysis Tools say. Static Analysis Tools often spot things that meet the above criteria, but that doesn’t mean it is right all the time. Adding comments to explain why something has been done the way it has when it isn’t obviously clear IS best practice. It is best to write clean code so you don’t need comments, but when there is always a clear benefit to adding them, you should add them.

I’m not even sure what the author meant by “open up code smells”. I can’t think of a Static Analysis rule that could violate. Recently, I have encountered many examples of developers writing bad code, or changing code they didn’t need to touch. Then when I challenge them on it, they say the Static Analysis tool was flagging it up, so they modified the code until it stopped complaining. The point of Static Analysis tools is to write good code/to enforce best practice. However, like the reviewing developer said, it isn’t always correct. Also, the rules aren’t smart enough to detect when you are trying to trick them, so you can write bad code that doesn’t violate the rules. If you are resorting to tricking the analysis, then you definitely don’t care about writing good code.

In the book Clean Code, a point that the author makes is that you can eradicate code comments by writing readable code. Sometimes that’s not possible which is why you then need to write a code comment to provide the clarity.