Coding Style – Aligning Comments with Code Elements

clean codecoding-stylecomments

I am a home, amateur developer for 25 years and I just had a bright idea regarding comments. Like all such novel bright ideas, someone has probably already done it and there is probably a consensus on its brightness.

I wrote a complex (to me at least) line of code and wanted to clearly comment it so that I can still understand it in a few days:

// get all tags for the line, or empty array
                                          // all tags available
                                                                    // filtered for tags that are set
                                                                                                 // do they incude the current tag of the line?
                                 // add the result of include to the container
(note.tags || []).forEach(tag => inc.push(Object.keys(this.allTags).filter(x => this.allTags[x]).includes(tag)))

Each comment is vertically aligned with the piece of the line it refers to. Is this something acceptable?

The obvious pro is that the comments actually relate to the piece that is being commented.

The cons include apocalyptic line reformatting (losing the match between the indentation and the piece being commented) and probably the surprise to the person reading the code if this is not typical.

Best Answer

No, such aligned comments are not a good practice. It is not clear that the comments relate to specific positions on the line. It just looks like very wildly formatted code.

The comment's indents will also be removed by an auto-formatter. Therefore, if you want to make comments about a specific part of the line, I'd put the spacing/offset within the comment:

(note.tags || []).forEach(tag => inc.push(Object.keys(this.allTags).filter(x => this.allTags[x]).includes(tag)))
//^^^^^^^^^^^^^^^                ^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^                       ^^^^^^^^^^^^^
// |                              |        |                         |                           do they incude the current tag of the line?
// |                              |        |                        filtered for tags that are set
// |                              |       all tags available
// |                             add the result of include to the container
// get all tags for the line, or empty array

However, this is still fairly unreadable because it's a very long line. It is also difficult to maintain, since tiny changes to the line would cause the ASCII art to get out of sync.

I sometimes use this strategy for very tricky code, in particular for documenting complex regular expressions.

A better solution is to split this expression over multiple lines, and to potentially use intermediate variables to make the code more self-documenting. The variable names can describe the intent of the value they're holding:

const allAvailableTags = Object.keys(this.allTags);
const tagsThatAreSet = allAvailableTags.filter(x => this.allTags[x]);
const tagsOnTheLine = note.tags || [];
// Check if the line tags are set.
// Add the result (true/false) to the `inc` container.
tagsOnTheLine.forEach(tag => {
  inc.push(tagsThatAreSet.includes(tag));
});

Note that extracting the constant expressions also happens to avoid the nested forEach/filter loop.

WoJ's answer also suggests splitting the expression over multiple lines, but without introducing extra variables.

But personally, I'd write it like this:

// Check if this line's tags are set.
for (const tag of (note.tags || [])) {
  inc.push(!!this.allTags[tag]);
}

This uses the !! boolification pseudo-operator that converts a value to true/false. Equivalently, you could use the Boolean(...) function. Your allTag object also allows easy checks to see whether a tag is set, without having to filter the keyset first. Seeing such connections can be easier when the code is well-formatted.

And as a general point, you might re-consider what is worth commenting. Every language has its libraries and idioms (small patterns). It is usually wasted effort to comment things that are very normal in the language, for example the object || default idiom, what the .filter() method does, or how !! works. Therefore:

  • Avoid writing very clever code – often a for-loop is clearer than functions like filter/map/reduce.
  • Focus comments on the intent, not on the implementation. The why, not the how, because implementation details are easy to look up later.
  • Bookmark a JavaScript reference like MDN to quickly look up language details you're not sure about.
  • Use an editor/IDE such as VS Code that can show a documentation popup when hovering over a function/variable.