javascript for loop does not iterate all the element

Question

I was trying to add a div for each element in the distinctCtyNames array. However if I press the button once, it only create a few divs and I have to press the button multiple time in order to pop all the elements and make them divs. When I use for(i=0;i<length;i++){ctyName = distinctCtyNames[i] /*and create div*/}, there is no such problem

my code is as following:

<input type="submit" name="BtnLogin" value="addDiv" onclick="addDiv()" id="addDiv" style="height:26px;width:100px;Z-INDEX: 105; LEFT: 440px; TOP: 72px"/>

and

console.log(distinctCtyNames);
for (ctyName in distinctCtyNames){
        var element = document.createElement('div');
        ctyName = distinctCtyNames.pop();
        element.id = ctyName;
        element.style['cssText'] = "width: 1000px; height: 400px; margin: 0px auto;"
        oContent.appendChild(element);
    }

Debug your code by stepping through it in a debugger. – Jul 09 '15 at 08:13 — , Jul 09 '15 at 08:13

score 3 · Accepted Answer · answered Jul 09 '15 at 08:11

You reassign the ctyName var inside the for loop by using the array.pop method.
pop will remove the LAST element in the array, while you iterate from the start, and when you remove the last element the array will change, thus not iterate the full array.
When you run your loop, the ctyName var will be set for each instance in the distinctCtyNames array.

 for (ctyName in distinctCtyNames) { ...

Could be written:

 for(var i=0,len=distinctCtyNames.length;i<len;i++) {
   var ctyName = distinctCtyNames[i];

Stop using the pop method in the loop, and it should work correctly.

score 1 · Answer 2 · edited May 23 '17 at 12:29

Problem 1: Reassigning values
You are iterating through the array using

for (ctyName in distinctCtyNames)

At the same time, you are removing elements from this array using and reassigning the ctyName value.

ctyName = distinctCtyNames.pop();

For example, you have an array:

['Toronto', 'Moscow', 'New York', 'Paris']

Your for-loop takes 'Toronto' and assigns it to ctyName. Now, in loop you make ctyName = distinctCtyNames.pop() and ctyName becomes equal to Paris. You have lost 'Toronto' value.

That's why your loop misses some values. You shouldn't do this.

Problem 2: Incorrect for usage.
Also, when you iterate through an array with for loop, it iterates through keys. You should get the value using the key.

It works fine:

for (ctyKey in distinctCtyNames) // changed to ctyKey
{
    var ctyName = distinctCtyNames[ctyKey]; // get ctyName using array[key]
    var element = document.createElement('div');
    element.id = ctyName;
    element.style['cssText'] = "width: 1000px; height: 400px; margin: 0px auto;"
    oContent.appendChild(element);
}

Here is the working JSFiddle Demo.

However, iterating through the array using for in is a bad practice.
Read more about it at this StackOverflow topic.

It should probably be `element.id = distinctCtyNames[ctyName];` to get the element and not the index. — JJJ, Jul 09 '15 at 08:09
No, given that `distinctCtyNames` is an array [this will **not** work fine](http://stackoverflow.com/q/500504/1048572). — Bergi, Jul 09 '15 at 08:10
@Bergi Sorry. I didn't notice that the OP has an error in this place. Fixed it. — Yeldar Kurmangaliyev, Jul 09 '15 at 08:16

javascript for loop does not iterate all the element

2 Answers2