3

I have this JavaScript code:

for (var idx in data) {
    var row = $("<tr></tr>");
    row.click(function() { alert(idx); });
    table.append(row);
}

So I'm looking through an array, dynamically creating rows (the part where I create the cells is omitted as it's not important). Important is that I create a new function which encloses the idx variable.

However, idx is only a reference, so at the end of the loop, all rows have the same function and all alert the same value.

One way I solve this at the moment is by doing this:

function GetRowClickFunction(idx){
    return function() { alert(idx); }
}

and in the calling code I call

row.click(GetRowClickFunction(idx));

This works, but is somewhat ugly. I wonder if there is a better way to just copy the current value of idx inside the loop?

While the problem itself is not jQuery specific (it's related to JavaScript closures/scope), I use jQuery and hence a jQuery-only solution is okay if it works.

Anurag
  • 140,337
  • 36
  • 221
  • 257
Michael Stum
  • 177,530
  • 117
  • 400
  • 535
  • I just wrote a blog post on this exact issue. The problem is JavaScript's weird scoping rules which cause the same `idx` to be bound to each function. Using an inline function to create a new scope as suggested by @Pointy is a good solution, IMHO. http://blogs.perl.org/users/mike_friedman/2010/03/i-need-closure.html – friedo Mar 04 '10 at 20:20
  • 1
    @Michael: Eric Lippert wrote a blog post about the exact same issue: "Closing over the loop variable considered harmful" (http://blogs.msdn.com/ericlippert/archive/2009/11/12/closing-over-the-loop-variable-considered-harmful.aspx). I believe it even was *you* who posted that on Twitter. ;) – Tomalak Mar 04 '10 at 20:45
  • @Tomalak Yup, C# has exactly the same "issue", but there it's trivial to solve by introducing a new local variable. JavaScript doesn't have block-scoping so that easy way doesn't work there sadly :( – Michael Stum Mar 04 '10 at 20:53
  • @Michael: Not sure what you mean. Introducing a new local variable is the solution for JavaScript, too. Both @EndangeredMassa's and @Pointy's solutions do *exactly that*. – Tomalak Mar 04 '10 at 20:57
  • 1
    @Tomalak The solution by @EndangeredMassa doesn't work as thisIdx is not local to the for-loop for local to the function. Even though it says "var thisIdx", i'm modifying the same thisIdx and putting it into the function. Pointy's solution works because the (function(idx)) block creates a new scope (and because it seems syntactically the best solution I've accepted it). Sadly, { and } are useless to scope variables in JavaScript :( – Michael Stum Mar 04 '10 at 21:03
  • 1
    @Michael: You are right, @EndangeredMassa's solution would need another closure around the callback. – Tomalak Mar 04 '10 at 21:17

2 Answers2

9

You could put the function in your loop:

for (var idx in data) {
  (function(idx) {
    var row = $("<tr></tr>");
    row.click(function() { alert(idx); });
    table.append(row);
  })(idx);
}

Now, the real advice I'd like to give you is to stop doing your event binding like that and to start using the jQuery "live" or "delegate" APIs. That way you can set up a single handler for all the rows in the table. Give each row the "idx" value as an "id" or a "class" element or something so that you can pull it out in the handler. (Or I guess you could stash it in the "data" expando.)

Pointy
  • 405,095
  • 59
  • 585
  • 614
  • @Pointy: What would keep me from using a single handler for all the rows with `click()`? All I'd have to do is define a function object beforehand and pass it in, so where is the actual advantage of `live()`? – Tomalak Mar 04 '10 at 20:55
  • Well, in this particular case I guess it might not make that much difference - you're building the table row by row anyway. It makes a difference when the table's already there (like, it was part of the original HTML), because then you effectively handle clicks on each row with one jQuery call (instead of one call per row, either externally or internally). – Pointy Mar 04 '10 at 21:17
2

Check out jquery's data() method:

http://api.jquery.com/jQuery.data/

Description: Store arbitrary data associated with the specified element.

majman
  • 1,993
  • 14
  • 23