Spot the bug - #127: Spell Shelf List

programming web dev
1

Why does my spell shelf only clear half the charms?

function purgeDarkSpells(shelfElement) {
  const charms = shelfElement.getElementsByClassName("cursed");
  
  for (let i = 0; i < charms.length; i++) {
    const item = charms[i];
    if (item.dataset.dangerLevel > 3) {
      shelfElement.removeChild(item);
    }
  }
  
  const remaining = shelfElement.querySelectorAll(".cursed").length;
  return `Purge complete! ${remaining} mild curses remain.`;
}

Reply with what is broken and how you would fix it.

2

The issue is that charms is a live HTMLCollection.

When you remove an item, the collection updates and shifts the indices. Your loop then skips the next element. To fix this, iterate backwards:

function purgeDarkSpells(shelfElement) {
  const charms = shelfElement.getElementsByClassName("cursed");
  for (let i = charms.length - 1; i >= 0; i--) {
    const item = charms[i];
    if (item.dataset.dangerLevel > 3) {
      shelfElement.removeChild(item);
    }
  }
  const remaining = shelfElement.querySelectorAll(".cursed").length;
  return `Purge complete! ${remaining} mild curses remain.`;
}

Alternatively, convert charms to a static array first using Array.from(charms) or the spread operator [...charms].

That way, the array won’t change during iteration.

3

Spot the Bug answer: The loop iterates over a live HTMLCollection while modifying it, causing elements to be skipped.

The fix:

Iterate backwards: `for (let i = charms.length - 1; i >= 0; i--)`

Why:
When an element is removed, the HTMLCollection automatically updates, shifting subsequent elements to lower indices. If iterating forwards, the next element to be processed will have skipped an index, effectively missing the element that moved into its original position.

First-answer leaderboard

  1. @kirupa - 5 (firsts) :trophy:
  2. @adnanahmed - 2 (firsts)
  3. @Apexcodes - 2 (firsts)
  4. @emmawalter5 - 1 (first)
4

This is a classic. I’ve seen similar issues in build scripts where files were being processed from a generated list that was also being modified. Very hard to debug if you’re not expecting it.

5

Look. Mutating a list while iterating over it is how you get burned. I’ve seen this exact thing cause issues in incident response scripts.

6

You’re right, modifying a list while iterating over it is a classic way to introduce bugs. The charms.length changes with each removal, so the loop skips elements.

To fix it, you could iterate backward or convert charms to a static array first. This article on iterating over collections has a good example of how to do that:

https://www.kirupa.com/html5/iterating_over_collections.htm

7

Hmm, or you could collect the items to remove into a separate list and then process the removals in a second pass. That keeps the iteration clean.