for loop vs forEach in JavaScript -- example from book

Question

I am currently learning JavaScript using O'Reilly's "Learning Web Application Development". In the example, we are constructing a website using HTML and CSS, which includes 3 tabs which should be able to be selected and become the "active" tab. The books claims that the following two ways of writing the tab code are equivalent:

1)

var main = function() {
    "use strict";
    var tabNumber;
    for (tabNumber=1;tabNumber<=3;tabNumber++) {
        var tabSelector = ".tabs a:nth-child("+tabNumber+") span";
        $(tabSelector).on("click",function() {
           $(".tabs span").removeClass("active");
           $(tabSelector).addClass("active");
           $("main .content").empty();
           return false;
        });
    }  
}
$(document).ready(main);

2)

var main = function() {
    "use strict";
    $(".tabs a span").toArray().forEach(function(element) {

        $(element).on("click", function() {
            console.print("this element: " + element);
            $(".tabs span").removeClass("active");
            $(element).addClass("active");
            $("main .content").empty();
            return false;
        });
    });
}
$(document).ready(main);

However, they do not output the same result. The version using forEach works correctly, so that when I click one of the tabs the attention moves to that tab and it becomes highlighted. However, in the version using a for loop, whenever I click any tab, the attention always moves to the last tab. To confirm what is happening, I printed out the name of the element inside the event listener with both methods, using 3 tabs total. And using the for loop, no matter which tab I click I am getting a response of

"this element: .tabs a:nth-child(3) span"

Could someone please help me explain why this is happening? Why is the output different using for or forEach? And why, using for, is it always passing the last element of tabs to the event listener?

The for loop goes through each <a>, whereas the forEach goes through each , in your code. — blex
– blex, Commented Jul 7, 2014 at 0:00
I did know that. Just to clarify I am not asking how to accomplish this task as I already have one valid solution using forEach. Rather I am hoping to understand the discrepancy between to methods that should be equivalent — Paul
– Paul, Commented Jul 7, 2014 at 0:24
@blex, thank you for your comments. In the for loop I am accesing the child of the <a> element. So aren'y I iterating over the in both cases? To clarify, there are 3 .tabs, and each has one <a> child element, which in turn each have 1 inline element. I want to iterate over each of the 3 elements. — Paul
– Paul, Commented Jul 7, 2014 at 0:38
pro-tip: If you ever get stuck on what code is doing, make sure to indent it properly. This makes it not only easier for you to read but also makes it easier for the people you're seeking help from to read. This is a good habit to get into now, as on larger projects you definitely must write readable, maintainable code. Hope this helps! — jamesmortensen
– jamesmortensen, Commented Jul 7, 2014 at 1:16

SharkofMirkwood · Accepted Answer · 2014-07-06 23:59:41Z

1

Looks like there's a problem here:

var tabSelector = ".tabs a:nth-child("+tabNumber+") span";
$tabSelector.on("click",function(){

You've created a variable that doesn't have the $ at the beginning, then attached the event to a variable (not sure what it would refer to) with a $ at the beginning.

It should be changed to this, I believe:

$(tabSelector).on("click",function(){

answered Jul 6, 2014 at 23:59

SharkofMirkwood

13k2 gold badges19 silver badges25 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

blex · Accepted Answer · 2014-07-07 01:04:16Z

0

In the for loop solution, you are setting tabSelector multiple times like so:

var tabSelector = ".tabs a:nth-child("+tabNumber+") span";

This selector is, in the end, going to be set to the last tabNumber, which is always going to be called when you make a reference to it:

$(tabSelector).addClass("active");

To avoid that, replace it by this, which will be different for each of them:

$(this).addClass("active");

JS Fiddle Demo

edited Jul 7, 2014 at 1:04

answered Jul 7, 2014 at 0:57

blex

25.7k6 gold badges48 silver badges78 bronze badges

3 Comments

Paul Over a year ago

Thank you, this solution worked. Surprised that there was an error in this book. So if I understand, what is happening is that the for loop calls 3 asynchronous event listeners at the start, so when the event occurs, the for loop has already been executed and tabSelector is set to 3. On the other hand the "this" keyword refers to the parent class to which the .on() function belongs, which is whatever tabSelector corresponded to the current loop at the time of the event. But "this" is also changing on every iteration, so after the looping is done why don't we have the same overwriting problem?

blex Over a year ago

@Paul I am surprised too, but a lot of programming books have mistakes in them that get noticed and fixed over time. The onClick functions are declared but not executed until you click on a tab. So by the time you click on one, it will not process what is in this variable yet. When you click, it sees that variable, and looks up what it references to (last tab). this is a variable created on the spot when you click on a tab, and as you said, it is a reference to the object you click on. Everytime you click, it will try to resolve this, and find the right selector for you.

Paul Over a year ago

OK, I think I understand: Although

Collectives™ on Stack Overflow

for loop vs forEach in JavaScript -- example from book

2 Answers 2

Comments

JS Fiddle Demo

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related