views:

902

answers:

5

See:

for (var i in this.items) {
    var item = this.items[i];
    $("#showcasenav").append("<li id=\"showcasebutton_"+item.id+"\"><img src=\"/images/showcase/icon-"+item.id+".png\" /></li>");
    $("#showcasebutton_"+item.id).click(function() {
        alert(item.id);
        self.switchto(item.id);
    });
}

The problem is that the alerted item.id is always the id of the last item in the array (this.items). How to solve?

A: 

try this loop

for (var i=0; i < this.items.length; i++) {
  this.items[i]
};
Jure C.
No, the loop is fine. The problem is that every added list item sees the same "item.id" in the closure.
Bart van Heukelom
+9  A: 

The problem you have here is that the variable item changes with each loop. When you are referencing item at some later point, the last value it held is used. You can use a technique called a closure (essentially a function that returns a function) to quickly scope the variable differently.

    for (var i in this.items) {
            var item = this.items[i];
            $("#showcasenav").append("<li id=\"showcasebutton_"+item.id+"\"><img src=\"/images/showcase/icon-"+item.id+".png\" /></li>");
            $("#showcasebutton_"+item.id).click( 
                // create an anonymous function that will scope "item"
                (function(item) {
                   // that returns our function 
                   return function() {
                    alert(item.id);
                    self.switchto(item.id);
                   };
                })(item) // immediately call it with "item"
            );
    }

A side note - I see that you have jQuery here. It has a helper function $.each() that can be used with arrays, and can be a shortcut for simple for/each loops. Because of the way the scoping works in this call - you wont need to use a closure because "item" is already the parameter of the function when it was called, not stored in a var in the parent function's scope, like was true in your example.

$.each(this.items,function(i, item) {
  $("#showcasenav").append("<li id=\"showcasebutton_"+item.id+"\"><img src=\"/images/showcase/icon-"+item.id+".png\" /></li>");
  $("#showcasebutton_"+item.id).click(function() {
    alert(item.id);
    self.switchto(item.id);
  });
});
gnarf
as ugly as that is, it does work
Bart van Heukelom
Found a more elegant solution for you after the fact - check out the advantage of using `$.each()`
gnarf
Thanks. I'll try it tomorrow
Bart van Heukelom
Thank you! I just spend a while trying to create a closure to use a local variable inside my click callback. The tricky part for me removing the parameters from the function being returned.
Sam
+1  A: 

The other way to approach this is to make sure the = items[i] business is effectively done by calling a function. In shorthand, this:

for (var i in this.items) {
    (function(item) {
        $("#showcasenav").append("<li id=\"showcasebutton_"+item.id+"\"><img src=\"/images/showcase/icon-"+item.id+".png\" /></li>");
        $("#showcasebutton_"+item.id).click(function() {
            alert(item.id);
            self.switchto(item.id);
        });
    })(this.items[i]);
}

The anonymous function there is a bit messy, making it rather preferable to have a not-so-anonymous one around for the purpose, but it does the trick.

VoteyDisciple
A: 

Javascript closures store references to their variables, so all of your onclick handlers are using the same variable.

You need to capture the variable in an intermediate function, like this:

function buildClickHandler(pageNumber) {
    return function()  {    //Create and return a new function
        alert(item.id);
        self.switchto(item.id);
    }
}

Then, use that function to create click handlers, like this:

for (var i in this.items) {
    var item = this.items[i];
    $("#showcasenav").append("<li id=\"showcasebutton_"+item.id+"\"><img src=\"/images/showcase/icon-"+item.id+".png\" /></li>");

    $("#showcasebutton_"+item.id).click(buildClickHandler(item));
}

Each call to buildClickHandler creates a separate closure that has its own variable.

SLaks
A: 

I know very well that this is an old post, but it seems that the genious people designing jQuery (which I figured you must be using) have made the most optimal solution to your problem, as I understand it.

In the new 1.4 version of the library, they have added the jQuery.proxy() function. This enables you to efficiently modify the context/scope of the function you are calling - done the jQuery way, which ensures that you can stop using techniques that could potentially mess up things.

Sune Rasmussen