What is the idiomatic way to iterate over a NodeList and move its elements without converting to an Array?

Question

This jsFiddle illustrates the problem. If I'm understanding what's going on correctly, as I iterate over and modify the NodeList in place, the counter variable i misses every other node.

In that fiddle, I have two lists, #one and #two and I'd like to move all the children of #one to #two--

<ol id='one'>
    <li class='move'>one</li>
    <li class='move'>two</li>
    <li class='move'>three</li>
    <li class='move'>four</li>
    <li class='move'>four</li>
</ol>

<ol id='two'>
</ol>

with some minimal JavaScript

var lisToMove = document.getElementsByClassName('move');
var destination = document.getElementById('two');
for (var i = 0; i < lisToMove.length; i++) {
    destination.insertBefore(lisToMove[i], null);
}

I know I can fix this by simply converting the NodeList to an Array and iterating over the Array instead, but I was wondering what the right way to iterate over a NodeList if you are modifying the NodeList itself (and not just the nodes) is?

Answer 1

Node lists are often implemented as node iterators with a filter. This means that getting a property like length is O(n), and iterating over the list by re-checking the length will be O(n^2).

var paragraphs = document.getElementsByTagName('p');
for (var i = 0; i < paragraphs.length; i++) {
  doSomething(paragraphs[i]);
}

It is better to do this instead:

var paragraphs = document.getElementsByTagName('p');
for (var i = 0, paragraph; paragraph = paragraphs[i]; i++) {
   doSomething(paragraph);
} 

This works well for all collections and arrays as long as the array does not contain things that are treated as boolean false.

In cases where you are iterating over the childNodes you can also use the firstChild and nextSibling properties.

var parentNode = document.getElementById('foo');
for (var child = parentNode.firstChild; child; child = child.nextSibling) {
  doSomething(child);
}

Answer 2

I'm not sure there's a "right" way. Whatever is easiest for you to implement, is easy to understand, and works, should be what you use. A simple slice of the NodeList shouldn't be a problem, even if there's the tiniest performance hit for simply creating a new array and copying references to its contents.

If you truly don't want to copy an array, you can loop over the NodeList backwards, guaranteeing that any changes you make to that node in the list won't actually affect the list.

Here's an example of what I mean:

var lisToMove = document.getElementsByClassName('move');
var destination = document.getElementById('two');
var i = lisToMove.length;
while (i--) {
    destination.insertBefore(lisToMove[i], destination.firstChild);
}

DEMO: http://jsfiddle.net/TYZPD/

As you can see, it's not a straightforward change in looping - the logic of where/how to insert the node has to change too, since everything is backwards. That's why I changed the second argument to insertBefore to be destination.firstChild. I'm sure there's other ways to handle the logic, but the principle is the same.

---

References:

• Node.firstChild - https://developer.mozilla.org/en-US/docs/Web/API/Node.firstChild
• Node.insertBefore - https://developer.mozilla.org/en-US/docs/Web/API/Node.insertBefore

Answer 3

Using an array is the correct way to do things like this. Just like if you ever have to modify an array while you loop over it, you probably want to separate the modification part and the loop part so one can't affect the other.

In this case, doing that is accomplished by converting the NodeList to an array.

As suggested by Benjamin Gruenbaum, you could also use querySelectorAll, which returns a frozen NodeList.

Demo here