JSLint for backwards for loops

Question

Douglas Crockford, and thus, JSLint really dislike for loops. Most of the time I agree, and use [].forEach and Object.keys(obj) to iterate through arrays or dictionaries.

However, there are cases where the built in functions are not better than a for loop in any reasonable sense (as far as I know), and I dislike having warnings on my JSLints.

One such case is, when one has a bunch of items on a canvas. When drawing, you want to iterate through the array forwards, but when hit testing a mouse click, you need to iterate backwards. E.g.

// ...
drawItems = function () {
    objects.forEach(drawItem);
},
hitTest = function (x, y) {
    var i;
    for (i = objects.length - 1; i >= 0; i -= 1) {
        if (test(objects[i], x, y)) {
            return objects[i];
        }
    }
}
// ...

I can think of a couple of ways to pass the test, but they are all clunky. E.g.

hitTest = function (x, y) {
    var item;

    objects.reverse();
    objects.some(function (v) {
        if (test(v)) {
            item = v;
            return true;
        }
    });
    objects.reverse();
    return item;
}

and

hitTest = function (x, y) {
    return objects.reduceRight(
        function (prev, curr) {
            return prev || (
                test(curr, x, y)
                    ? curr
                    : prev
            );
        },
        undefined
    );
}

Which will pass JSLint happily, but, to be honest, look disgusting. I don't like the idea of "hacking" my code just to make it pass a test.

Other than choosing the "tolerate for statement" option (because that affects the linting of the rest of the code), how would I deal with this whilst avoiding wasteful operations like reversing or copying the array?

Answer 1

Ran into an interesting, reverse-free alternative fix here -- see the 3rd & 4th -- while looking at something else today. Easy enough to swap from jQuery's each to forEach or every. You have to get the length for your original for anyhow, so actually pretty clean, though a little hipster.

/*jslint white:true, devel:true */
var a = [3,4,5,6], max = a.length-1;
a.forEach(function (ignore, i) {
    "use strict";
    console.log(a[max-i]);
});

I'm a little worried about performance, since you're pulling out elements twice with each iteration, once at a[max-i], and once with the forEach's callback's (here, throwaway) third argument, though maybe that's a micro-optimization concern.

Using forEach only for the indicies is what's setting off the hipster warning. Ironic, since Crockford has shown a stark dislike for hipsters in the past, so I'm not sure he'd like this "fix" either -- none of us may have answered your "meta-question". Tempted to bug Crockford's list; this is a good and interesting question, but you don't always get a good answer on that list.

But that's a pretty (deceptively?) clean looking workaround.

Answer 2

Since I'm of the opinion that you shouldn't de-optimize your code just to make a pointless warning go away, here are some of your options for dealing with this:

1. Ignore the misguided warning.
2. Switch to jsHint which allows you to instruct jsHint to ignore certain sections of code (with embedded comments).
3. Isolate the code that jsLint is unhappy with in a function or a method so the warning only occurs once in your entire source rather than every time you want to do a reverse traverse.

For example, here's a reverse traverse function:

// traverse an array in reverse order
// if the callback returns true, iteration will be stopped
function reverseForEach(array, fn) {
    for (var i = array.length - 1; i >= 0; i--) {
        if (fn(array[i], i, array) === true) {
            return;
        }
    }
}

Or, as a method on Array prototype:

// traverse an array in reverse order
// if the callback returns true, iteration will be stopped
Array.prototype.reverseForEach = function(fn) {
    for (var i = this.length - 1; i >= 0; i--) {
        if (fn(array[i], i, this) === true) {
            return;
        }
    }
}

Answer 3

You can slice the array so that your hitTest doesn't have any potential side-effects.

hitTest = function (x, y) {
    return objects.slice(0).reverse().find(function(v) {
        return test(v);
    });
}

This way you don't have to worry that you'll forget to re-reverse the array.