36

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?

Anurag
142k37 gold badges223 silver badges262 bronze badges
asked Aug 26, 2009 at 0:36
0

5 Answers 5

40

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);
 });
});
answered Aug 26, 2009 at 0:46
Sign up to request clarification or add additional context in comments.

2 Comments

Found a more elegant solution for you after the fact - check out the advantage of using $.each()
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.
4

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.

answered Aug 26, 2009 at 0:46

Comments

1

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.

answered Aug 26, 2009 at 0:49

Comments

0

try this loop

for (var i=0; i < this.items.length; i++) {
 this.items[i]
};
Nick Dandoulakis
43.3k17 gold badges106 silver badges139 bronze badges
answered Aug 26, 2009 at 0:41

1 Comment

No, the loop is fine. The problem is that every added list item sees the same "item.id" in the closure.
0

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.

answered Feb 25, 2010 at 0:01

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.