CODEWITHSUNDEEP
javascriptjquery
Nadir Muzaffar
Nadir Muzaffar

Reputation: 4842

Javascript for loop error

This is what I have:

function get_bad_changelist_authors() {
   var changelistAuthorDivs = $('div.bad_changelist_author');
   var changelistAuthors = [];

   for (var div in changelistAuthorDivs) {
      changelistAuthors.push($(changelistAuthorDivs[div]).text());
   }

   return changelistAuthors;
}

changeListAuthorDivs has n elements but for some reason it iterates n+1 times and initialized div to a string (ie. the string "length") on the last iteration. On the last iteration is where this loop has its error.

Upvotes: 0

Views: 271

Answers (3)

Eliu
Eliu

Reputation: 767

Try this

 for (var div in changelistAuthorDivs) {
     if(Object.prototype.hasOwnProperty.call(changelistAuthorDivs[div], div))
     changelistAuthors.push($(changelistAuthorDivs[div]).text());
  }

Upvotes: 0

Felix Kling
Felix Kling

Reputation: 816334

It seems you are using jQuery (please confirm). Then you can use .map() [docs] to achieve the same in a more concise way:

function get_bad_changelist_authors() {
   return $('div.bad_changelist_author').map(function() {
        return $(this).text();
   }).get();
}

Alternatively , use .each() [docs] if you deal with jQuery objects.

In general, @James is completely right, don't use for...in to iterate over arrays (see the warning here). In this case, the jQuery object has to be treated like an array.

Upvotes: 2

James Allardice
James Allardice

Reputation: 165951

JavaScript for...in loops will also iterate over properties of the object (length is a property of the object, which is why you end up with a div containing that string), which is not what you want. Use a normal for loop instead:

for(var i = 0; i < changeListAuthorDivs.length; i++) {
   //Do stuff
}

Upvotes: 1

Related Questions