[javascript] What does the JSLint error 'body of a for in should be wrapped in an if statement' mean?

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?

This question is related to javascript jslint

The answer is


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


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


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!


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]);
  }
}

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.


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
});

@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]);
    }
}