SomeKittens
SomeKittens

Reputation: 39522

.forEach loop: use variable

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]}

Upvotes: 4

Views: 19709

Answers (3)

Erik  Reppen
Erik Reppen

Reputation: 4645

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.

Upvotes: 1

MaxArt
MaxArt

Reputation: 22647

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}
            ...
        });
    });
});

Upvotes: 10

jagm
jagm

Reputation: 576

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++;
});

Upvotes: 1

Related Questions