With native JavaScript, I intend to traverse a collection of elements in the DOM that contain a link and an image (and possibly other elements). The image may or may not be inside the link—in most cases the image will be in the link, but in some cases it may be directly adjacent to the link, or it may be a child of another div or span that is adjacent to its relevant link. However, both the link and the image will always be contained within a container element, of which there may be several.
For example, envision a stock photo gallery arranged in a grid layout. The HTML for each item in the gallery's collection may look like any one of the snippets below (simplified for cleanliness and brevity).
It might be:
<div class="item">
<a href="...">
<img src="..." />
</a>
</div>
Or it may be:
<div class="item">
<img src="..." />
<a href="...">Image Title</a>
</div>
Or it may be:
<div class="item">
<span data-blah="...">
<a href="...">Image Title</a>
</span>
<span data-meh="...">
<img src="..." />
</span>
</div>
Note: I can't control the given HTML. Nevertheless, I would like to traverse and manipulate the DOM flexibly enough to handle any instance of HTML structure given above. To do so efficiently, I'd first need to build the collection of container elements before traversing, yes?
So this:
var items = document.querySelectorAll('.item');
for(i in items) { ... } // more efficient
rather than this:
for(i in document.querySelectorAll('.item')) { ... } // less efficient
as the latter would need to re-query the .item
selector again inside the loop, which is inefficient. Right?
However, to manipulate the link and image of each item being looped, I still need to access the link and image of said item. To do that, I could use querySelector()
on the .item
like so:
var items = document.querySelectorAll('.item');
for(i in items) {
var link = items[i].querySelector('a'), img = items[i].querySelector('img');
// manipulate link and img elements
}
But is triggering querySelector()
twice per loop-instance the most efficient approach?
Is there a more efficient approach to access the link and image of each .item
element being looped-through than by calling querySelector()
twice on each looped .item
element?
Consider that the primary intent of manipulation is to change the link URL to the image source URL and add some attributes to the link and/or image. Seems simple enough. I just want to make sure the loop logic is efficient. Ideally, the logic will ultimately be used in either a bookmarklet or a browser plugin. The collection of .item
elements could potentially number in the thousands, so I'm hoping to keep this traversal and manipulation process as efficient as possible.
1 Answer 1
You should not iterate over an array or array-like object using a foreach loop,
you should use a good old-fashioned for
loop instead:
var items = document.querySelectorAll('.item');
for (var i = 0; i < items.length; ++i) { ... }
Yes, this sequential for
loop doesn't look as elegant as a foreach loop,
but unfortunately, using a foreach loop for arrays in JavaScript is incorrect.
As for this:
var items = document.querySelectorAll('.item');
for (var i = 0; i < items.length; ++i) {
var item = items[i];
var link = item.querySelector('a');
var img = item.querySelector('img');
}
Having two .querySelector
lookups in each iteration is not really a problem,
because the lookups are done on a small object, a tiny subset of the entire document.
I wrote the loop body slightly differently from your original:
I put the value of
items[i]
in a localitem
variable. This is to avoid duplicatingitem[i]
. It's good to eliminate duplications, because if you ever need to change something, you need to do it in all duplicates, which is potentially error-prone.I declared
link
andimg
on two lines. This is slightly more verbose, but a bit clearer.
-
\$\begingroup\$ I was considering asking if a for loop would be better. But why is it better (other than for the sake of backwards compatibility)? I like the syntax of the for-in better than the for loop. I think for loops are ugly. But this is about efficiency, so if they're more efficient then I suppose it should be used. Also, I didn't think there'd be much performance benefit redeclaring items[i] as item on its own. Is there? What about declaring each var individually? Why not on one line as I did in my example? \$\endgroup\$purefusion– purefusion2015年01月19日 21:06:14 +00:00Commented Jan 19, 2015 at 21:06
-
\$\begingroup\$ Hi @purefusion, I updated my answer to address the questions in your comment \$\endgroup\$janos– janos2015年01月19日 21:23:53 +00:00Commented Jan 19, 2015 at 21:23
-
\$\begingroup\$ Thanks, I'm aware that for loops are intended for arrays, but technically speaking, querySelector() and related methods return objects, not arrays (according to
typeof
). So I figured a for-in/foreach loop would be fine. Or do arrays get returned as objects fromtypeof
too? Didn't think to check that. \$\endgroup\$purefusion– purefusion2015年01月19日 21:27:28 +00:00Commented Jan 19, 2015 at 21:27 -
\$\begingroup\$ @purefusion you really need to carefully read that linked post. It explains it very nicely. As for what
typeof
returns, take a look attypeof([])
ortypeof(new Array())
\$\endgroup\$janos– janos2015年01月19日 21:33:29 +00:00Commented Jan 19, 2015 at 21:33
Explore related questions
See similar questions with these tags.
for (i in document.querySelectorAll('.item')) { ... }
is less efficient is erroneous.querySelectorAll
would not get called once per iteration of the loop. \$\endgroup\$for
loop, notfor..in
. Also note you forgot to declarei
. \$\endgroup\$querySelectorAll
would not get called in the loop, how would I access each.item
element when inside the loop, according your argument? \$\endgroup\$...
part that has to be less efficient if you writefor (i in document.querySelectorAll('.item')) { ... }
. \$\endgroup\$