6

This is not a dup of JSlint: unexpected 'for' , please do not mark it as such:

There is no native for loop for looping through an object or object literal ( I know there is a .forEach for arrays ).

Why does jslint suggest not using for loops? How do you loop through an object that does not have Array.forEach() or Array.some() or similar?

But the bigger more important question, is why is this suggestion made?

Obviously I could use Object.keys() and then forEach() but this seems like a longer way to do it.

Originating Code

// on a truthy match returns true and breaks from loop
Pub.someKey = function (obj, func, con) {
    var key;
    if (!Pub.isFunction(func)) {
        return false;
    }
    for (key in obj) {
        if (obj.hasOwnProperty(key)) {
            if (func.call(con, obj[key], key, obj)) {
                return true;
            }
        }
    }
    return false;
};
Pub.forSomeKey = Pub.someKey;
Community
  • 1
  • 1
  • 3
    *why is this suggestion made?* Because `for...in` will iterate through properties all the way up the prototype chain (that's why you include the `obj.hasOwnProperty` part). Crockford has a lot of very informative things to say about Javascript, but his word is not law. You can turn that particular warning off in jslint I believe. – Matt Burland May 10 '16 at 20:43
  • Just tell that JSlint thingie to shut up and use a `for` loop. – Oriol May 10 '16 at 20:43
  • @MattBurland is right; you might also see http://jshint.com/docs/options/#forin – Andrew Burgess May 10 '16 at 20:44
  • It's worth mentioning that ES6 gives you the ability to define custom iteratable objects - [for...of](https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Statements/for...of) – stefanhorne May 10 '16 at 20:50
  • 1
    @Shaffanhoon Only iterable objects. Of course, you can define `Object.prototype[Symbol.iterator] = function*() { for(let key of Object.keys(this)) yield [key, this[key]]; };` to make (almost) all objects iterable. – Oriol May 10 '16 at 20:50
  • @MattBurland That's there in JS*H*int, I believe, but no dice in JS*L*int. You can set the `for` directive, but you'll still get `Expected 'Object.keys' and instead saw 'for in'` in this case. [See answer, below](http://stackoverflow.com/a/37190218/1028230). – ruffin May 12 '16 at 14:43
  • Possible duplicate of [Loop through JavaScript object](http://stackoverflow.com/questions/684672/loop-through-javascript-object) –  May 12 '16 at 14:43
  • @JarrodRoberson Not really. That's simply a question about how to loop. **It's not a dupe, as that question doesn't care about linting** (no linting tags). Note: The nearly 2k vote answer there does not lint in JSLint (though [this answer's construction](http://stackoverflow.com/a/5737136/1028230) does). – ruffin May 12 '16 at 14:44

2 Answers2

3

I believe the answer you are looking for is linked in the accepted answer to the question you linked: http://www.jslint.com/help.html#forin

does not recommend use of the for in statement. Use Object.keys instead.

It goes on to explain why that is recommended.

The for in statement allows for looping through the names of all of the properties of an object. Unfortunately, it also loops through all of the properties that were inherited through the prototype chain. This has the bad side effect of serving up method functions when the interest is in data properties. If a program is written without awareness of this situation, then it can fail.

The body of every for in statement should be wrapped in an if statement that does filtering. It can select for a particular type or range of values, or it can exclude functions, or it can exclude properties from the prototype. For example,

for (name in object) {
    if (object.hasOwnProperty(name)) {
        ....
    }
}

Note that the above code will fail if the object contains a data property named hasOwnProperty. Use Object.keys instead.

ruffin
  • 16,507
  • 9
  • 88
  • 138
Jack A.
  • 4,245
  • 1
  • 20
  • 34
1

I'm not quite sure what you mean by "but this seems like a longer way to do it." Since you get to remove the hasOwnProperty check, it's actually, in some ways, much cleaner.

Here's an edited version that lints:

/*jslint browser */
/*global Pub */

// on a truthy match returns true and breaks from loop
Pub.someKey = function (obj, func, con) {
    "use strict";
    if (Pub.isFunction(func)) {
        Object.keys(obj).forEach(function (key) {
            if (func.call(con, obj[key], key, obj)) {
                return true;
            }
        });
    }
    return false;
};
Pub.forSomeKey = Pub.someKey;

(I have a mild dislike of multiple return values, but edited this to use them to more closely match your original. And it gets rid of the "no breaks in forEach" issue I mentioned in the original.)


For more on why JSLint doesn't like for, see this Crockford video Setzer22 linked (in this question). It's interesting watching, and gives a little more context on why he doesn't like for... in specifically:

I've never liked for... in because it does that stupid thing where it dredges through the prototype and you get all the methods and stuff that you didn't want. So now we've got Object.keys, which returns an array of stuff and you don't get the dredge, and then pass that to forEach and its brothers, and it works great. So I don't use for... in any more either.

Bonus tip: Watching Crockford at .5 speed while transcribing is hilarious, at least for a few minutes.


Aside: Unfortunately, @Oriol's and Matt Burland's comments don't hold, as I imagine @YeWhoseNameCantBeAtted found out. You can add the for directive to use standard for (i=0;...), but you'll still get Expected 'Object.keys' and instead saw 'for in' in this use case.

Community
  • 1
  • 1
ruffin
  • 16,507
  • 9
  • 88
  • 138