I'm looping over an array and modifying the contents of the array, but I don't get the results I expect.

Question

I'm looping over an array and modifying the contents of the array, but I don't get the results I expect. What am I missing or doing wrong?

I have two groups of divs (one with class attacker, and other enemy) with three elements each. I am trying to select one element from each side by making a border around it. Now i want to toggle classes from a attacker to enemy and the other way. But when I use for loop it somehow ignores some elements and changes only one or two div classes. Here is my code:

HTML:

<div id="army1">
    <div class="attacker">
        <img src="img/Man/Archer.jpg" />
        <div class="hp"></div>
    </div>
    <br><div class="attacker">
        <img src="img/Man/Knight.jpg" />
        <div class="hp"></div>
    </div>
    <br><div class="attacker">
        <img src="img/Man/Soldier.jpg" />
        <div class="hp"></div>
    </div>
    <br>            
</div>

<div id="army2">
    <div class="enemy">
        <img src="img/Orcs/Crossbowman.jpg" />
        <div class="hp"></div>
    </div>
    <br><div class="enemy">
        <img src="img/Orcs/Mine.jpg" />
        <div class="hp"></div>
    </div>
    <br><div class="enemy">
        <img src="img/Orcs/Pikeman.jpg" />
        <div class="hp"></div>
    </div>
    <br>            
</div>

And my javascript code:

var attacker = document.getElementsByClassName('attacker');
var enemy = document.getElementsByClassName('enemy');


var button = document.getElementById("fight");

// var class1 = document.getElementsByClassName("first")[0].getAttribute("class");
// class1 = class1.split(" ");

//choose attacker
for (var i = 0; i < attacker.length; i++) {
    attacker[i].onclick = function () {
        //select only one attacker and set its id to attackerid
        if (this.getAttribute('class') != 'attacker first') {
            resetAttackerClasses();
            this.setAttribute('class', 'attacker first');
        } else {
            resetAttackerClasses();
        }

    };
}

//choose enemy
for (var i = 0; i < enemy.length; i++) {
    enemy[i].onclick = function () {
        //select only one attacker and set its id to enemyid
        if (this.getAttribute('class') != 'enemy second') {
            resetEnemyClasses();
            this.setAttribute('class', 'enemy second');
        } else {
            resetEnemyClasses();
        }
    };
}

//fight
button.onclick = function() {
    //take off enemy health
    document.getElementsByClassName('enemy second')[0].children[1].style.width = '50px';

    resetAttackerClasses();
    resetEnemyClasses();

     for (var i = 0; i < attacker.length; i++) {
            attacker[i].setAttribute('class', 'enemy');
            enemy[i].setAttribute('class', 'attacker');
    };
};


function resetAttackerClasses() {
    for (var i = 0; i < attacker.length; i++) {
        attacker[i].setAttribute('class', 'attacker');
    };
}
function resetEnemyClasses() {
    for (var i = 0; i < attacker.length; i++) {
        enemy[i].setAttribute('class', 'enemy');
    };
}

Answer 1

It's because you're removing the class that was used to fetch the element, which means the element will automatically be removed from the live NodeList (since it no longer matches the query).

When this happens, the NodeList is reindexed, so the next element becomes the current one, and you end up skipping over it with the next i++;

To fix it, iterate in reverse instead.

If you don't want to go in reverse, then decrement the index every time you remove an element from the list.