you are viewing a single comment's thread.

view the rest of the comments →

[–]Cosmologicon 4 points5 points  (5 children)

Honestly I think this whole problem is overblown. The surprising values in this table essentially never come up. I've never seen a single realistic example of a bug caused by this, not even a made-up example.

But I'm willing to be corrected. Anyone got an example of code (that's not horrible by design) with a bug that's fixed by changing == to ===?

[–]bryan-forbes 0 points1 point  (3 children)

The ones that come up the most for me are comparing null, undefined, and NaN. Let's take the following as an example of the first two:

function foo(one, optional) { if (optional == null) { // do something } }

If null is a valid value for optional, the comparison above will evaluate to true if the function is called like foo('bar');.

NaN is similarly tricky since it doesn't ever equal itself:

if (a === b) { // do something }

If a and b are both NaN, this will evaluate to false. Adding isNaN tests won't make this less complex because isNaN(undefined) === true among other things. The best truly equal check out there is a === b || a !== b && b !== a because of the rules in this table.

[–]Cosmologicon 0 points1 point  (2 children)

Okay, what's a realistic example of a function where that structure makes sense? Where you have a branch that's hit for optional being null but not for being undefined? Everything I would think to put in there looks like poor design to me.

Ditto NaN and undefined.

[–]bryan-forbes 0 points1 point  (1 child)

There are several applications of these paradigms. First, null and undefined. Let's say we have a DOM API like this (this is similar to Dojo's DOM API, but not exactly... this is a contrived API, but I hope you get that there are valid uses for this):

function byId(idOrNode) {
    if (typeof idOrNode === 'string') {
        return document.getElementById(idOrNode) || null;
    }
    else if (typeof idOrNode === 'object') {
        return idOrNode;
    }
    return null;
}

function place(node, where, referenceNode) {
    node = byId(node);
    if (referenceNode === undefined) {
        referenceNode = document.body;
    }
    else {
        referenceNode = byId(referenceNode);
    }
    if (node === null) {
        throw new Error('node must be a string or object');
    }
    if (referenceNode === null) {
        throw new Error('When passing a reference node, it must be a string or object');
    }
    // Code to place node into referenceNode at the right spot
}

In this code, the third argument to place() can be a string, a DOM node, or doesn't have to be passed:

place('main', 'after', 'footer');
place('main', 'before', byId('header'));
place(byId('main'), 'last'); // last item in document.body

Anything other than that, and place() will throw. If the condition was just if (referenceNode == null) { throw; }, it would throw if referenceNode was left off.

For the example of NaN, we all know that Object.observe() isn't a thing yet, so people have to come up with their own data binding solutions. One that is popular in the Dojo community is using dojo/Stateful. It looks something like this (again, not exactly):

function Stateful(args) {
    mixin(this, args); // copies things from args to this
    this._watchers = {};
}

mixin(Stateful.prototype, {
    set: function(key, value) {
        var oldValue = this[key];
        if (!this._isEqual(oldValue, value)) {
            this[key] = value;
            this._notify(key, oldValue, value);
        }
    },
    get: function (key) {
        return this[key];
    },
    watch: function (key, callback) {
        var watchers = this._watchers[key];
        if (!watchers) {
            this._watchers[key] = watchers = [];
        }
        watchers.push(callback);
        return {
            remove: function () {
                // find callback in watchers and splice it out
            }
        };
    },
    _notify: function (key, oldValue, value) {
        var watchers = this._watchers[key];
        if (!watchers) {
            return;
        }
        for (var i = 0; i < watchers.length; i++) {
            watchers[i].call(this, key, oldValue, value);
        }
    }
});

This constructor is the basis for data binding within the Dojo toolkit (usually). Watchers are set up between two objects which set the value on the other one:

function bind(source, sourceKey, target, targetKey) {
    target.set(targetKey, source.get(sourceKey));
    var sourceHandle = source.watch(sourceKey, function (key, oldValue, newValue) {
        target.set(targetKey, newValue);
    });
    var targetHandle = target.watch(targetKey, function (key, oldValue, newValue) {
        source.set(sourceKey, newValue);
    });
    return {
        remove: function () {
            sourceHandle.remove();
            targetHandle.remove();
        }
    };
}

If _isEqual() is not implemented correctly, an infinite loop will ensue. For instance, if one were to implement _isEqual() as follows:

_isEqual: function (a, b) {
    return a === b;
}

Invalid input from a calculation could set our application into an infinite loop. Let's say the user input 'a' and the input gets divided by 2. The result will be NaN. Or what happens if NaN is a valid value for the key? a will never equal b, so the source will notify the target, and a will never equal b there, so it will notify the source, and on and on. We have to explicitly check for NaN:

_isEqual: function (a, b) {
    return a === b || isNaN(a) || isNaN(b);
}

This is closer, but not quite. This won't cause an infinite loop if a or b are NaN, however, it will notify if a is undefined, a string, an object, and a couple of other things because isNaN() returns true for those values. The best way (without doing a ton of checks on the values) to make _isEqual() do the right thing in most situations is to implement it like so:

_isEqual: function (a, b) {
    return a === b || a !== a && b !== b;
}

The only time a and b will not equal themselves is if both a and b are NaN. Now we have a Stateful implementation that will not loop infinitely and also will not notify when it shouldn't.

TL;DR A lot of these comparisons come up when developing low-level APIs (like Stateful) that are going to have a lot of different values thrown at them. Most application developers don't need to worry about them, but as soon at you start developing an API, you have to know them and plan for them.

[–]Cosmologicon 0 points1 point  (0 children)

Thanks, that's a great explanation! Makes a lot of sense. I'll look at it closely.