Inadvertently breaking composability

July 2015

When you work with classes in JavaScript (that is, approximations of classes), everything tends to become a class. Have a look at this simple and contrived implementation of a recursive factorial function:

// MathUtil.js
var MathUtil = {
    fac: function (n) {
        if (n <= 1) return 1;

        return n * this.fac(n-1);
    }
}

return MathUtil;

Don’t do this. This is not a class. This is an object, with different types of values (presumably mostly functions) and there is no need to call this. The problem is that you are assuming that this is bound correctly. This is a big (and unnecessary) assumption to make, and really, it is putting the onus on the user for your own sloppy (or ignorant) implementation.

Let’s test it. A simple console will do.

MathUtil.fac(3);
// → 6

[1, 2, 3].map(MathUtil.fac);
// → Uncaught TypeError: this.fac is not a function

Sure. We can use it in a map in other ways. This is the most common way I see when doing code reviews:

[1, 2, 3].map(function (n) {
    return MathUtil.fac(n);
});
// → [1, 2, 6]

So much boilerplate! There’s also this next one. I like it more, because it clearly acknowledges the problem.

[1, 2, 3].map(MathUtil.fac.bind(MathUtil));
// → [1, 2, 6]

The solution here, is just to swap out this for MathUtil.

// MathUtil.js
var MathUtil = {
    fac: function (n) {
        if (n <= 1) return 1;

        return n * MathUtil.fac(n-1);
    }
}

return MathUtil;

That’s it. Problem solved.

[1, 2, 3].map(MathUtil.fac);
// → [1, 2, 6]

It is maybe not too surprising that these mistakes occur. Making code composable is difficult, and this is notoriously troublesome to work with in JavaScript, but this post is not about this, it is about constructing code to minimise surprises (that is, minimise knowledge required to use the code correctly). Here is another (rather infamous) example:

var a = [ 0,  1,  2,  3,  4,
          5,  6,  7,  8,  9,
         10, 11, 12, 13, 14,
         15, 16, 17, 18, 19];

a.map(parseInt)
// → [  0, NaN, NaN, NaN, NaN,
      NaN, NaN, NaN, NaN, NaN,
       10,  12,  14,  16,  18,
       20,  22,  24,  26,  28]

What happens here is that the second argument (the index) passed to the callback (parseInt) in the map is used as the radix from which to calculate the new number.

Let’s return to our factorial function for a second. It is recursive, so maybe we should structure it in such a way that takes advantage of the (soon to come) tail call optimization in browsers. Let’s give it a shot:

// MathUtil.js
var MathUtil = {
    fac: function (n, accu) {
        accu = arguments.length < 2 ? 1 : accu;

        if (n <= 1) return accu;

        return MathUtil.fac (n-1, n*accu);
    }
};

return MathUtil;

Looks great, doesn’t it? No! No it does not. Did you fall for it? I hope you didn’t. We introduced the very same problem that we eliminated earlier.

MathUtil.fac(1);             // → 1
MathUtil.fac(2);             // → 2
MathUtil.fac(3);             // → 6
[1, 2, 3].map(MathUtil.fac); // → [0, 2, 12]

The solution, is to use the closure provided to us when modularising the code.

// MathUtil.js
function recursiveFac (n, accu) {
    if (n <= 1) return accu;

    return recursiveFac(n-1, n*accu);
}

return {
    fac: function (n) {
        return recursiveFac(n, 1);
    }
};

Or, in case we’re in ES6 land:

// MathUtil.js
const recursiveFac = (n, accu) =>
    n <= 1 ? accu : recursiveFac(n-1, n*accu);

export const fac = n => recursiveFac(n, 1);

Good luck!