Loop of a list of items with javascript

Question

I read that to improve the performance of jquery should avoid use because some things are slowing down a bit. Among these is the method each() and that it would be appropriate to use a for loop ... (according to the web article I read, the method each() takes roughly 10 times greater than for loops javascript)..

I'm trying to used js but I have some problems: /

Jquery

$('#block-system-main-menu li').each(function () {      
        var text= ($(this).children('a').text());
        if (text == 'Map' || text== 'Vul' || text== 'Equa'){
            $(this).children('a').append('<span style="float:right; margin-right: 15%; line-height: inherit;" class="fa fa-chevron-right"></span>');
        }                   
    });

Javascript

var voce= $('#block-system-main-menu li');
for(var i=0; i< voce.length; i++) {
        var text= (voce[i].children('a').text());           
        if (text == 'Map' || text== 'Vul' || text== 'Equa'){
            voce[i].children('a').append('<span style="float:right; margin-right: 15%; line-height: inherit;" class="fa fa-chevron-right"></span>');
        } 
    }

but the loop does not work and I do not understand why ....

Thanks!

When reading things like this, bear in mind that 10 times as slow as something that's very fast is still a lot faster than other things that might be slowing your page down. I wouldn't be surprised if the `append` within the loop is far far slower than just doing the `.each`. — James Thorpe, Oct 26 '15 at 14:22
`but the loop does not work` what does it do? How is it "not working"? Is it giving you errors? Does it do nothing at all? Does it do something other than what you expected? — Matt Burland, Oct 26 '15 at 14:23
This question has been asked in the past. Check out http://stackoverflow.com/questions/11887450/each-vs-for-loop-and-performance — Will, Oct 26 '15 at 14:24
In the for loop example `voce[i].children('a')` should throw an error, as a Node doesn't have `cildren()` method: http://jsfiddle.net/pap5nxp7/ `Uncaught TypeError: voce[0].children is not a function`. — pawel, Oct 26 '15 at 14:24
Your first method is Jquery, your second one is Javascript. jQuery is a Javascript library. You can't just assume that because jquery implements something that javascript will do the same. — Liam, Oct 26 '15 at 14:27
@JamesThorpe i read this here http://code.tutsplus.com/tutorials/10-ways-to-instantly-increase-your-jquery-performance--net-5551 — Borja, Oct 26 '15 at 14:29
Apart from everything else that has been said here, you are doing a `$(this).children('a')` twice in the loop. So you will be creating a jQuery object and then getting its children both twice. Cache it. — Abhitalks, Oct 26 '15 at 14:32
@user3162975 Indeed. My comment still stands. Also note that that article was written in 2009 and that the performance chart doesn't even mention Chrome (and its javascript engine that kickstarted the race to improve JS performance) — James Thorpe, Oct 26 '15 at 14:33
I agree with @JamesThorpe. `jQuery.each` vs. JS `for` should not be a performance bottleneck, especially since `each` implementation uses a `for` loop: http://james.padolsey.com/jquery/#v=1.11.2&fn=jQuery.each - of course some additional checks and using a function call instead of loop body may slow it down, but I wouldn't spend time on such optimization. — pawel, Oct 26 '15 at 14:34

Matt Burland · Accepted Answer · 2015-10-26T14:48:50.843

Your problem is here. Compare:

var text= ($(this).children('a').text());

with:

var text= (voce[i].children('a').text());

In the first case, you are creating a jquery object with $(this). In the second case you aren't. voce[i] gives you an HTML element which does NOT have a function called children. So you should get an error there. To make it work, you'd need to create a jquery object from voce[i]. Something like:

var text = $(voce[i]).children('a').text();

Or as @GuerasArnaud suggests in his answer:

var text= voce.eq(i).children('a').text();

However, in all likelihood the for versus each probably isn't what's slowing your code down. You should actually profile your code and identify the bottlenecks in your code.

While when you read "x is slower than y" it's something that you should certainly keep in mind, you should also try to understand where it's applicable and actually important. I wouldn't completely avoid each in all cases, because sometimes the convenience and readability of your code is going to be more important that a micro-optimization of your codes performance in a non-critical section of code.

Also note that in your linked tutorial, you've latched on to the point of for vs each while missing the point of Cache. ALWAYS. So in your jquery code you are creating the $(this) object twice (although it's not quite as important in this case because you aren't using a selector and it depends on how often your if clause evaluates to true, but it's still a cost that gains you nothing in return).

So your loop would be better if you had this:

$('#block-system-main-menu li').each(function () {  
    var children = $(this).children('a');
    var text = children.text();
    if (text == 'Map' || text== 'Vul' || text== 'Equa'){
        children.append('<span style="float:right; margin-right: 15%; line-height: inherit;" class="fa fa-chevron-right"></span>');
    }                   
});

Or for the for loop:

var voce= $('#block-system-main-menu li');
for(var i=0; i< voce.length; i++) {
    var children = $(voce[i]).children('a');
    var text = children.text();           
    if (text == 'Map' || text== 'Vul' || text== 'Equa'){
        children.append('<span style="float:right; margin-right: 15%; line-height: inherit;" class="fa fa-chevron-right"></span>');
    } 
}

But again, unless voce.length is very large I doubt you'll find any significant difference in real-world performance.

score 1 · Answer 2 · answered Oct 26 '15 at 14:27

1

voce is a jQuery object but voce[i] is a HTMLNode Element, and it doesn't have a .children() method. If you want to stay in 'jquery like' you should use .eq()

var text= voce.eq(i).children('a').text();

answered Oct 26 '15 at 14:27

Arnaud Gueras

2,014
11
14

score 0 · Answer 3 · answered Oct 26 '15 at 15:28

So I have benchmarked the jQuery each against fixed JS for loop with jQuery methods and added a vanilla JS solution: http://jsperf.com/jquery-each-for-loop

On my machine jQuery.each vs JS for are very close to each other with vanilla JS outperforming both by quite a large margin.

The code without jQuery:

var voce = [].slice.call( document.querySelectorAll('#block-system-main-menu li'));
/* prepare the span to use .appendChild instead of innerHTML += */
var span = document.createElement('span');
    span.style.cssText = 'float:right; margin-right: 15%; line-height: inherit';
    span.className = 'fa fa-chevron-right';

for(var i=0;i<voce.length;i++){
   /* get the textContent with innerText fallback */
   var text = voce[i].textContent || voce[i].innerText;
   if (text == 'Map' || text == 'Vul' || text == 'Equa') {
       /* forEach to keep compatibility with .children('a') method which returns a collection. 
          Could be optimized further if you know you need to modify a single element */
       [].forEach.call( voce[i].querySelectorAll('a'), function(a){
           a.appendChild( span.cloneNode() );
       })
   }
}

Loop of a list of items with javascript

3 Answers3