.forEach loop: use variable

Question

I'm looping through a set of 16 ids and assigning an eventListener to each one. I want to send a number to my php file (1 for the first id, 2 for the second, etc, etc), but it seems that i is more dynamic than I'd like it to be. Every id sends 17.

klasses.forEach(function(klass){
    var svgElement = svgDoc.getElementById(klass); //get the inner element by id
    svgElement.addEventListener("mouseup",function(){
        $.ajax({
            type: "POST",
            url: "buildService.php",
            data: { "service" : i}
        }).done(function(msg){
            alert(lameArray[i]);
            $("#modalSpan").html(msg);
            $("#modmodal").modal();
        });
    });
    i++;
});

How can I set each one to a specific number? I've also tried:

var lameArray = [1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16];
...
data: { "service" : lameArray[i]}

Read Javascript closure property.... – scusyxx Jul 11 '12 at 22:09 — scusyxx, Jul 11 '12 at 22:09

MaxArt · Accepted Answer · 2012-07-11T22:14:11.473

What's i? The problem is that i is a global variable, or a variable outside the forEach cycle anyway, so when the mouseup event is triggered, the value of i in that istant is used, and not the one it had when the event listener is defined.

Mind you, since you're using forEach, that the callback function actually is called with a second parameter, which is the counter. So you can use:

klasses.forEach(function(klass, i) {
    ...
});

Now, i is a variable in the forEach scope, and serves your purposes. (forEach calls the callback function with a third parameter too, that is the collection itself - klasses in your case.)

Note: since you're using jQuery, you should code with a more "jQuery-like" style. So change your code in something like this:

$.each(klasses, function(i, klass) {
    $("#" + klass).mouseup(function(){
        $.ajax({
            type: "POST",
            url: "buildService.php",
            data: {service: i + 1}
            ...
        });
    });
});

score 1 · Answer 2 · answered Jul 11 '12 at 22:07

1

try this:

klasses.forEach(function(klass){
    (function(i) {
        var svgElement = svgDoc.getElementById(klass); //get the inner element by id
        svgElement.addEventListener("mouseup",function(){
            $.ajax({
                type: "POST",
                url: "buildService.php",
                data: { "service" : i}
            }).done(function(msg){
                alert(lameArray[i]);
                $("#modalSpan").html(msg);
                $("#modmodal").modal();
            });
        });
    })(i);
    i++;
});

answered Jul 11 '12 at 22:07

jagm

576
4
6

on the other hand, you can change first line in your code to `klasses.forEach(function(klass, i){` – jagm Jul 11 '12 at 22:08
1

Why don't you use a for-loop instead of forEach? – Bergi Jul 11 '12 at 22:10
1

What Bergi said. And if you end up calling a function at every iteration, then you can just use `forEach`, using the second parameter in the callback function as a counter. – MaxArt Jul 11 '12 at 22:18
@Bergi: Firstly, I used a part of code from the question. Secondly, in my opinion forEach has more friendly syntax than for-loop. And yes, I know that forEach is not supported by all browsers. – jagm Jul 11 '12 at 22:25
@MaxArt: I know, so I added first comment. – jagm Jul 11 '12 at 22:27

Erik Reppen · Answer 3 · 2012-07-11T23:26:12.103

Don't use forEach when you need an iterator index. And avoid JQuery's .each. It's completely unnecessary the vast majority of the time and it fires a callback function on every iteration so it's much slower in IE. You can write a perfectly lazy/compact loop with while.

var outerI = klasses.length;

while(outerI--){
    (function(i){
        i+=1;//doesn't affect outerI and you wanted 1-length so we add one.
        //I would personally just add 1 but it also adds clarity to the example

        //crap inside your forEach loop but without the i++
    })(outerI)
}

What was happening: You were telling an event listener to reference i from an outer scope. So whatever i was when that event kicked in is what you got.

Solution: Pass i's value into the scope of a function where it becomes a local var. The business with the parens is just a lazy way to define, evaluate and execute an anonymous function in what seems like one step. The function gets evaluated via the first parens (making it fireable) so the second set is like the arg you put into the inner 'i' param of the function definition. You're locking the value you want by passing it to a new local var basically.

Note on the while loop: while(0) evaluates as false, stopping the loop. This is weird if you think about it because length to 1 is one less than what you want. With while(i--) however i gets evaluated, then i is decremented so inside the block you get length-1 to 0 which is perfect for array notation. To decrement i before other operators hit it you'd typically do --i but it works out handily in lazy/efficient while loops.

.forEach loop: use variable

3 Answers3

Linked