Washing your code: avoid comments

#javascript #programming #cleancode

You’re reading an excerpt of my upcoming book on clean code, “Washing your code: write once, read seven times.” Preorder it on Leanpub or read a draft online.

Some developers never comment their code, some comment too much. The former believe that the code should be self-documenting, the latter read somewhere that they should always comment their code.

Both are wrong.

I don’t believe in self-documenting code. Yes, we should rewrite unclear code to make it more obvious, and use meaningful and correct names, but there are things that can’t be experessed by the code alone.

Commenting too much isn’t helpful either: comments start to repeat the code, and instead of helping to understand it, they introduce noise and repetition.

Getting rid of comments (or not)

There’s a popular technique of avoiding comments: when we want to explain a block of code in a comment, we should move this piece of code to its own function instead.

It’s often a good idea to extract complex calculations and conditions used inside an already long line of code:

if (
  platform.toUpperCase().indexOf('MAC') > -1 &&
  browser.toUpperCase().indexOf('IE') > 1 &&
  this.wasInitialized() &&
  this.resize > 0
) {
  return true;
}

Here, we could extract the conditions into their own functions or variables with meaningful names:

const isMacOs = platform.toUpperCase().includes('MAC');
const isIE = browser.toUpperCase().includes('IE');
const wasResized = this.resize > 0;
if (isMacOs && isIE && this.wasInitialized() && wasResized) {
  return true;
}

Now, the condition is shorter and more readable, because names help us to understand what the condition does in the context of the code.

However, I don’t think that splitting a linear algorithm, even a long one, into several functions, and then calling them one after another, makes code more readable. Jumping between functions (and even more so – files) is harder than scrolling, and if we have to look into functions’ implementations to understand the code, then the abstraction wasn’t the right one. Naming could be a problem too, when all the extracted functions are parts of the same algorithm.

Overall, I don’t like when the code is measured by its physical metrics, like number of lines. Long functions aren’t always hard to read and modify, And the really complex code could be tiny. We talk about code splitting in more detail in the Divide and conquer, or merge and relax chapter.

Good comments

Comments are useful to answer why code is written in a certain, often mysterious, way:

If the code is fixing a bug or is a workaround for a bug in a third-party library, a ticket number or a link will be useful.
If there’s an obvious simpler alternative solution, a comment should explain why this solution doesn’t work in this case.
If different platforms behave differently, and the code accounts for this, it should be also mentioned in a comment.

Such comments will save us from accidental “refactoring” that makes code easier but removes some necessary functionality or breakes it for some users.

High-level comments, explaining how code works, are useful too. If the code implements an algorithm, explained somewhere else, a link to that place would be useful.

And any hack should be explained in a HACK or FIXME comment:

  // HACK: Importing defaultProps from another module crashes Storybook Docs,
  // so we have to duplicate them here
  static defaultProps = {
    label: '',
  }

TODO comments are okay (more like okayish) too, if they contain a ticket number when something will be done. Otherwise they are just dreams that will likely never come true. Unless a dream is exactly what we want to document: a desire that the code was doing more than it does – error handling, special cases, supporting more platforms, minor features, and so on – but it wasn’t implemented due to, probably, lack of time.

// TODO: On React Native it always returns DEV, since there's no actual location available
const getEnvironment = (hostname = window.location.hostname) => {
  if (hostname.includes('qa.')) {
    return Environment.QA;
  }
  if (hostname.includes('example.com')) {
    return Environment.PROD;
  }
  return Environment.DEV;
};

Idea: Maybe we should start using DREAM comments for such cases...

Bad comments

We’ve talked about useful comments. However, there are many more kinds of comments that we should never write.

Probably the wost kind of comments are comments explaining how code works. They either repeat the code in a more verbose language, or explain language features:

// Fade timeout = 2 seconds
const FADE_TIMEOUT_MS = 2000;

// This will make sure that your code runs
// in the strict mode in the browser
'use strict';

Code comments isn’t the best place to teach teammates how to use certain language features. Code reviews, pair programming sessions, and team documentation would be more suitable and efficient.

Next, fake comments: they pretend to explain some decision, but actually they don’t explain anything, and othen blame someone else for poor code and tech debt:

// force 24 hours formatting for Chinese and Korean
const hour12 = locale === 'zh' || locale === 'ko' ? false : undefined;

I see lots of these comments in one-off design "adjustments". For example, a comment will say that there was a design requirement to use a non-standard color but it won’t explain why it was required and why none of the standard colors worked in that case:

.shareButton {
  color: #bada55; // Using non-standard color to match design
}

And by lots I mean really plenty:

// Design decision

// This is for biz requirements

// Non-standard background color needed for design

// Designer's choice

// Using non-standard color to match design

Requirement is a very tricky and dangerous word. Often what’s treated as a requirement is just a lack of education and collaboration between developers, designers, and project managers. If we don’t know why something is required, we should always ask, and the answer could be flabbergasting!

There may be no requirement at all, and we can use a standard color from the project theme:

.shareButton {
  color: $text-color--link;
}

Or there may be a real reason to use a non-standard color, that we may put into a comment:

$color--facebook: #3b5998; // Facebook brand color
.shareButton {
  color: $color--facebook;
}

In any case it’s our responsibility to ask why as many times as necessary.

Same with comments that explain conditions: there may be no need for a special case, and we could remove the whole condition with its comment. See more in the Avoid conditons chapter.

Start thinking about: