I recently Googled ‘Best way to document JavaScript’, and one of the first results was this blog, which I thought contained some good info, as well as some things I don’t agree with. Top of the list would be:

Self-documenting code is bullshit.

I mean… it’s not… is it?

Self-Documenting Code

Self-documenting code is code written in such a way that it’s easy to read and understand its intention. It doesn’t make all formal documentation redundant - even a system made of perfectly self-documenting code can benefit from documentation describing its high-level structures and goals - but it removes the need for a lot of documentation and code comments.

For Example

The blog gives the following example of code documented with comments:

/**
 * Toggle visibility of a content tab
 * @param  {String} selector Selector for the element
 * @param  {Node}   toggle   The element that triggered the tab
 */
var toggleVisibility = function (selector, toggle) {

    // If there's no selector, bail
    if (!selector) return;

    // Get the tab to show
    var elem = document.querySelector(selector);
    if (!elem) return;

    // Show the element
    elem.classList.add('active');

    // If a toggle element was provided, add an .active class 
    // for styling
    if (toggle) {
        toggle.classList.add('active');
    }

    // Bring the newly visible element into focus
    elem.focus()

    // If elem.focus() didn't work, add tabindex="-1" and try 
    // again (elements that aren't focusable by default need a 
    // tabindex)
    if (document.activeElement.matches(selector)) return;
    elem.setAttribute('tabindex', '-1');
    elem.focus();
};

The code itself is pretty neatly-written - let’s take it line-by-line, consider which comments add value, and if we can make it more self-documenting.

JSDoc Documentation

The function is declared with JSDoc documentation:

/**
 * Toggle visibility of a content tab
 * @param  {String} selector Selector for the element
 * @param  {Node}   toggle   The element that triggered the tab
 */
var toggleVisibility = function (selector, toggle) {

According to the documentation, this function toggles the visibility of a tab - but its name and the names of its arguments make no mention of tabs. Maybe the file containing this function is tab-specific, so within its context this makes sense, but maybe not. In any case, this can be more self-documenting:

var toggleTabVisibility = function (tabSelector, triggerElement) {

The function name and selector argument now mention tabs, which makes it more clear what this function is for, and what to pass in. I’ve also renamed toggle to triggerElement, as according to the documentation, toggle is the ‘element that triggered the tab’. With these changes, I’d argue the JSDoc block is redundant.

Redundant Comments

Next statement:

    // If there's no selector, bail
    if (!tabSelector) return;

This is one of those comments which basically repeats its code. It’s the sort of comment Uncle Bob recommends not to write in Clean Code. It’s noise, and we can remove it.

Generic Names and Lies

Next statement:

    // Get the tab to show
    var elem = document.querySelector(tabSelector);
    if (!elem) return;

This statement is already a little easier to understand as selector is now named tabSelector, but it can be improved. elem is a completely generic variable name - it gives us a hint what type the variable is, but tells us nothing about its purpose.

Secondly, is the comment lying? It says we’re getting the tab to ‘show’, but doesn’t this function toggle visibility? The JSDoc said ‘Toggle visibility of a content tab’ - I’d expect from that it would show hidden tabs, and hide visible ones. Sure enough though, it only shows tabs, it doesn’t hide them. This is one of the problems with documentation and comments - unlike code, they can lie.

So with a quick detour to rename the function:

var showTab = function (tabSelector, triggerElement) {

…we can make the comment redundant with a descriptive variable name:

    var tabToShow = document.querySelector(tabSelector);
    if (!tabToShow) return;

Self-Documenting CSS

Next statement:

    // Show the element
    tabToShow.classList.add('active');

It’s not 100% clear that adding the active class will cause tabToShow to become visible, but with a more self-documenting CSS class name:

    tabToShow.classList.add('visible');

…the comment is redundant.

The comments for the next two statements again pretty much repeat what their code is doing, especially with our updated naming:

    // If a toggle element was provided, add an .active class 
    // for styling
    if (triggerElement) {
        triggerElement.classList.add('visible');
    }

    // Bring the newly visible element into focus
    tabToShow.focus()

They’re noise, and we can remove them.

Worthwhile Comments

The comment for the next statements actually adds value:

    // If elem.focus() didn't work, add tabindex="-1" and try 
    // again (elements that aren't focusable by default need a 
    // tabindex)
    if (document.activeElement.matches(tabSelector)) return;
    tabToShow.setAttribute('tabindex', '-1');
    tabToShow.focus();

It explains how we ended up at the final two lines, and why we’re adding a tabindex - that information would be tricky to convey in the code itself, so the comment has earned its place.

We can however, self-document a little more with a helper function:

function hasFocus(elementSelector) {
    return document.activeElement.matches(elementSelector);
}

...

    if (hasFocus(tabSelector)) return;
    
    // tabToShow.focus() didn't work, add tabindex="-1" and try 
    // again (elements that aren't focusable by default need a 
    // tabindex)
    tabToShow.setAttribute('tabindex', '-1');
    tabToShow.focus();

…the slightly-altered comment remains though, and still serves a purpose.

Self-Documented

Our final, refactored function looks like this:

var showTab = function (tabSelector, triggerElement) {

    if (!tabSelector) return;

    var tabToShow = document.querySelector(tabSelector);
    if (!tabToShow) return;

    tabToShow.classList.add('visible');

    if (triggerElement) {
        triggerElement.classList.add('visible');
    }

    tabToShow.focus()

    if (hasFocus(tabSelector)) return;

    // tabToShow.focus() didn't work, add tabindex="-1" and try 
    // again (elements that aren't focusable by default need a 
    // tabindex)
    tabToShow.setAttribute('tabindex', '-1');
    tabToShow.focus();
};

With more self-documenting function, parameter and variable names, the documentation and almost every comment is redundant.

No bullshit!