What does the JSLint error 'body of a for in should be wrapped in an if statement' mean?
Asked Answered
L

9

281

I used JSLint on a JavaScript file of mine. It threw the error:

for( ind in evtListeners ) {

Problem at line 41 character 9: The body of a for in should be wrapped in an if statement to filter unwanted properties from the prototype.

What does this mean?

Linehan answered 26/12, 2009 at 10:49 Comment(2)
By default 'in' iterates over inherited properties as well. Usually, the body is wrapped in if (evtListeners.hasOwnProperty(ind)) to restrict processing only to own (non-inherited) properties. Still, in some cases you really want to iterate over all properties, including the inherited ones. In that case, JSLint forces you to wrap the loop body in an if statement to decide which properties you really want. This will work and make JSlint happy: if (evtListeners[ind] !== undefined)Hardecanute
Most answers are outdated. an updated solution can be found at https://mcmap.net/q/36096/-loop-through-an-array-in-javascriptOccupancy
H
469

First of all, never use a for in loop to enumerate over an array. Never. Use good old for(var i = 0; i<arr.length; i++).

The reason behind this is the following: each object in JavaScript has a special field called prototype. Everything you add to that field is going to be accessible on every object of that type. Suppose you want all arrays to have a cool new function called filter_0 that will filter zeroes out.

Array.prototype.filter_0 = function() {
    var res = [];
    for (var i = 0; i < this.length; i++) {
        if (this[i] != 0) {
            res.push(this[i]);
        }
    }
    return res;
};

console.log([0, 5, 0, 3, 0, 1, 0].filter_0());
//prints [5,3,1]

This is a standard way to extend objects and add new methods. Lots of libraries do this. However, let's look at how for in works now:

var listeners = ["a", "b", "c"];
for (o in listeners) {
    console.log(o);
}
//prints:
//  0
//  1
//  2
//  filter_0

Do you see? It suddenly thinks filter_0 is another array index. Of course, it is not really a numeric index, but for in enumerates through object fields, not just numeric indexes. So we're now enumerating through every numeric index and filter_0. But filter_0 is not a field of any particular array object, every array object has this property now.

Luckily, all objects have a hasOwnProperty method, which checks if this field really belongs to the object itself or if it is simply inherited from the prototype chain and thus belongs to all the objects of that type.

for (o in listeners) {
    if (listeners.hasOwnProperty(o)) {
       console.log(o);
    }
}
 //prints:
 //  0
 //  1
 //  2

Note, that although this code works as expected for arrays, you should never, never, use for in and for each in for arrays. Remember that for in enumerates the fields of an object, not array indexes or values.

var listeners = ["a", "b", "c"];
listeners.happy = "Happy debugging";

for (o in listeners) {
    if (listeners.hasOwnProperty(o)) {
       console.log(o);
    }
}

 //prints:
 //  0
 //  1
 //  2
 //  happy
Hexad answered 26/12, 2009 at 11:17 Comment(9)
You should not use for in to iterate over arrays because the language does not garauntee the order in which for in will enumerate over an array. It might not be in numeric order. In addition, if you use the `for(i=0;i<array.length;i++) style construct, you can be sure that you're only iterating numeric indexes in order, and no alphanumeric properties.Recipe
I found myself looking at this answer again because I was convinced that this bit of JSLint was broken. I had code that was roughly: for (o in listeners) { if (listeners.hasOwnProperty(i)) { console.log(o); } } The problem is that I had a bug, I'd switched variable names i to o and missed a reference. JSLint is smart enough to make sure that you're checking hasOwnProperty for the correct property on the correct object.Scintillant
for in, however, is fine to iterate over an object's property. The OP never said the for in was applied to an Array. The hasOwnProperty is best practice, however there are cases where you don't want it - for example if an object extends another, and you want to list both the objects and the 'parent' one's properties.Vary
Wow, I thought this video (destroyallsoftware.com/talks/wat) was overstating fundamental issues with the javascript language, but it seems that it was just scratching the surface.Sivan
I think that instead of scaring people away from for-in loops (which are awesome, by the way), we should educate them how they work (done properly in this answer) and introduce them to Object.defineProperty() so they can safely extend their prototypes without breaking anything. Incidentally, extending native objects' prototypes should not be done without Object.defineProperty.Keg
Actually, wrong. You can use "for in" in some cases, like creating a key, value object. var myObj = Object.create(null). This new object will not have any prototype, therefore, no unwanted properties will show up in the "for in" interation.Lamkin
The irony here is that TSLint and ESLint both warn you for using for(;;) loops, telling you to use for...in loops instead. Then further warns you for not using the if filter inside of it...Analyzer
Does JavaScript guarantee that in the classic for loop, it will start with the own properties of the object? Convoluted scenarios are imaginable.Write
I think in place of listeners.hasOwnProperty(o) you should use Object.prototype.hasOwnProperty.call(listeners, o).Arne
R
94

Douglas Crockford, the author of jslint has written (and spoken) about this issue many times. There's a section on this page of his website which covers this:

for Statement

A for class of statements should have the following form:

for (initialization; condition; update) {
    statements
}

for (variable in object) {
    if (filter) {
        statements
    } 
}

The first form should be used with arrays and with loops of a predeterminable number of iterations.

The second form should be used with objects. Be aware that members that are added to the prototype of the object will be included in the enumeration. It is wise to program defensively by using the hasOwnProperty method to distinguish the true members of the object:

for (variable in object) {
    if (object.hasOwnProperty(variable)) {
        statements
    } 
}

Crockford also has a video series on YUI theater where he talks about this. Crockford's series of videos/talks about javascript are a must see if you're even slightly serious about javascript.

Recipe answered 26/12, 2009 at 11:41 Comment(0)
D
21

Bad: (jsHint will throw a error)

for (var name in item) {
    console.log(item[name]);
}

Good:

for (var name in item) {
  if (item.hasOwnProperty(name)) {
    console.log(item[name]);
  }
}
Disvalue answered 10/9, 2015 at 11:9 Comment(0)
C
8

Honestly adding a whole line just to check if the key exist while using a syntax which is supposed to iterate over the keys of an object makes for .. in useless. Just use Object.keys(obj).forEach(key => {} instead.

Cetinje answered 29/10, 2021 at 13:37 Comment(1)
After eslint started alerting me of this problem when using for..in while trying to iterate through object keys, I stop using for..in completely and moved to using Object.keys instead with for...ofElasmobranch
C
7

Vava's answer is on the mark. If you use jQuery, then the $.each() function takes care of this, hence it is safer to use.

$.each(evtListeners, function(index, elem) {
    // your code
});
Chunk answered 30/1, 2010 at 6:32 Comment(2)
If performance is any consideration here, I wouldn't recommend using $.each (or underscore.js's _.each) if you can get away with the raw for loop. jsperf has a few eye-opening comparison tests that are worth running.Molecular
This ( jsperf.com/each-vs-each-vs-for-in/3 ) is more realistic because it employs the basic proto filterHooten
N
7

@all - everything in JavaScript is an object (), so statements like "only use this on objects" are a bit misleading. In addition JavaScript is not strongly typed so that 1 == "1" is true (although 1 === "1" is not, Crockford is big on this). When it comes to the progromatic concept of arrays in JS, typing is important in the definition.

@Brenton - No need to be a terminology dictator; "associative array", "dictionary", "hash", "object", these programming concepts all apply to one structure in JS. It is name (key, index) value pairs, where the value can be any other object (strings are objects too)

So, new Array() is the same as []

new Object() is roughly similar to {}

var myarray = [];

Creates a structure that is an array with the restriction that all indexes (aka keys) must be a whole number. It also allows for auto assigning of new indexes via .push()

var myarray = ["one","two","three"];

Is indeed best dealt with via for(initialization;condition;update){

But what about:

var myarray = [];
myarray[100] = "foo";
myarray.push("bar");

Try this:

var myarray = [], i;
myarray[100] = "foo";
myarray.push("bar");
myarray[150] = "baz";
myarray.push("qux");
alert(myarray.length);
for(i in myarray){
    if(myarray.hasOwnProperty(i)){  
        alert(i+" : "+myarray[i]);
    }
}

Perhaps not the best usage of an array, but just an illustration that things are not always clearcut.

If you know your keys, and definitely if they are not whole numbers, your only array like structure option is the object.

var i, myarray= {
   "first":"john",
   "last":"doe",
   100:"foo",
   150:"baz"
};
for(i in myarray){
    if(myarray.hasOwnProperty(i)){  
        alert(i+" : "+myarray[i]);
    }
}
Noonberg answered 10/12, 2010 at 19:41 Comment(2)
"Only use this on objects" means don't use it on Arrays or anything else that extends Object, otherwise, as you point out, it would be very silly since everything extends ObjectOria
'"associative array", "dictionary", "hash", "object", these programming concepts all apply to one structure in JS.' No. They are different concepts from different languages, with similarities to each other. But if you go about assuming that they are /exactly the same/ and to be used in the same way, for the same purposes, you are setting yourself up for making some really stupid mistakes you could avoid by being less ignorant about how the language you're using works.Recipe
Y
3

Surely it's a little extreme to say

...never use a for in loop to enumerate over an array. Never. Use good old for(var i = 0; i<arr.length; i++)

?

It is worth highlighting the section in the Douglas Crockford extract

...The second form should be used with objects...

If you require an associative array ( aka hashtable / dictionary ) where keys are named instead of numerically indexed, you will have to implement this as an object, e.g. var myAssocArray = {key1: "value1", key2: "value2"...};.

In this case myAssocArray.length will come up null (because this object doesn't have a 'length' property), and your i < myAssocArray.length won't get you very far. In addition to providing greater convenience, I would expect associative arrays to offer performance advantages in many situations, as the array keys can be useful properties (i.e. an array member's ID property or name), meaning you don't have to iterate through a lengthy array repeatedly evaluating if statements to find the array entry you're after.

Anyway, thanks also for the explanation of the JSLint error messages, I will use the 'isOwnProperty' check now when interating through my myriad associative arrays!

Yingyingkow answered 19/4, 2010 at 14:54 Comment(4)
You are deeply confused. There is no such thing as "associative arrays" in javascript. That is strictly a php concept.Recipe
It's true that these objects don't have a length property, but you can do it another way: var myArr = []; myArr['key1'] = 'hello'; myArr['key2'] = 'world';Zygotene
@Nyuszika7H That is the wrong way. If you don't need the integer indexed Array, you should not use var myArr = [], it should be var myArr = {} in PHP they are the same thing, but not in JS.Oria
The associative "array" isn't an array.Sadiras
I
0

This means that you should filter the properties of evtListeners with the hasOwnProperty method.

Itemized answered 26/12, 2009 at 11:9 Comment(0)
H
0

Just to add on to the topic of for in/for/$.each, I added a jsperf test case for using $.each vs for in: http://jsperf.com/each-vs-for-in/2

Different browsers/versions handle it differently, but it seems $.each and straight out for in are the cheapest options performance-wise.

If you're using for in to iterate through an associative array/object, knowing what you're after and ignoring everything else, use $.each if you use jQuery, or just for in (and then a break; once you've reached what you know should be the last element)

If you're iterating through an array to perform something with each key pair in it, should use the hasOwnProperty method if you DON'T use jQuery, and use $.each if you DO use jQuery.

Always use for(i=0;i<o.length;i++) if you don't need an associative array though... lol chrome performed that 97% faster than a for in or $.each

Hagbut answered 20/5, 2011 at 5:40 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.