8

I'm using jQuery and I have a strange thing that I don't understand. I have some code:

for (i = 1; i <= some_number; i++) {
    $("#some_button" + i).click(function() {
        alert(i);
    });
}

"#some_button" as the name says - they are some buttons. When clicked they should pop-up a box with it's number, correct? But they don't. If there is 4 buttons, they always pop-up "5" (buttons count + 1). Why is that so?

Anurag
  • 140,337
  • 36
  • 221
  • 257
dmitq
  • 137
  • 1
  • 5
  • It is a very common problem as most answers suggest. Also, all questions tagged `javascript`, `closures,` and `loops` will point to this exact problem and solutions - [javascript + closures + loops](http://stackoverflow.com/questions/tagged/javascript+closures+loops), including this one now :) – Anurag Jun 30 '10 at 15:47
  • we need a way for SO to have a huge pop-up saying "THIS HAS BEEN ANSWERED BEFORE", with links to the right question. I've seen at least 3 javascript versions and 2 python versions asking this question. – Claudiu Jun 30 '10 at 15:52
  • 1
    @Claudiu: Unfortunately it's very difficult for newcomers to JavaScript/Python to search for these kinds of problems. – Daniel Vassallo Jun 30 '10 at 16:04
  • Related: http://stackoverflow.com/questions/227820 http://stackoverflow.com/questions/512166/ – Josh Lee Jul 08 '10 at 19:45

6 Answers6

18

It has to do with JavaScript scoping. You can get around it easily by introducing another scope by adding a function and having that function call itself and pass in i:

for (var i = 1; i <= some_number; i++) {
  (function(j) {
    $("#some_button" + j).click(function() {
      alert(j);
    });
  })(i);
}

This creates a closure - when the inner function has access to a scope that no longer exists when the function is called. See this article on the MDC for more information.

EDIT: RE: Self-calling functions: A self-calling function is a function that calls itself anonymously. You don't instantiate it nor do you assign it to a variable. It takes the following form (note the opening parens):

(function(args) {
  // function body that might modify args
})(args_to_pass_in);

Relating this to the question, the body of the anonymous function would be:

$("#some_button" + j).click(function() {
  alert(j);
});

Combining these together, we get the answer in the first code block. The anonymous self-calling function is expecting an argument called j. It looks for any element with an id of some_button with the integer value of j at the end (e.g. some_button1, some_button10). Any time one of these elements is clicked, it alerts the value of j. The second-to-last line of the solution passes in the value i, which is the loop counter where the anonymous self-calling function is called. Done another way, it might look like this:

var innerFunction = function(j) {
  $("#some_button" + j).click(function() {
    alert(j);
  });
};

for (var i = 1; i <= some_number; i++) {
  innerFunction(i);
}
Hooray Im Helping
  • 5,194
  • 4
  • 30
  • 43
  • dear god :( Could you please, pretty please, rewrite it in a more readable manner? Nobody who doesn't already know the answer will ever understand a thing from the piece you wrote. `);});})(i);}` ... – SF. Jun 30 '10 at 14:22
  • 1
    @SF: this is pretty much what you gotta do.. Maybe this will help: you turn `for (...) { LOGIC(i); }`, where `LOGIC` is arbitrary code operating on the variable `i`, into `for (...) { (FUNC)(i) }`, immediately creating a function and giving it `i`. `FUNC` is simply `function (j) { LOGIC(j); }`. – Claudiu Jun 30 '10 at 15:55
  • Yes, you can think of a closure as a function that has a reference to every outer variable it uses, rather than a copy. This bug arose because i was used by reference inside the closure. This fix resolves it by copying the value to j - by passing it into a function as an argument. – Chris Moschini Dec 27 '11 at 23:29
9

You are having a very common closure problem in the for loop.

Variables enclosed in a closure share the same single environment, so by the time the click callback is called, the loop will have run its course and the i variable will be left pointing to the last entry.

You can solve this with even more closures, using a function factory:

function makeOnClickCallback(i) {  
   return function() {  
      alert(i);
   };  
} 

var i;

for (i = 0; i < some_number; i++) {
    $("#some_button" + i).click(makeOnClickCallback(i));
}

This can be quite a tricky topic, if you are not familiar with how closures work. You may to check out the following Mozilla article for a brief introduction:

Daniel Vassallo
  • 337,827
  • 72
  • 505
  • 443
2

Because in the moment you click them, i == 5.

Matteo Mosca
  • 7,380
  • 4
  • 44
  • 80
  • heh very succinct, but this will only help someone who already knows what the problem is – Claudiu Jun 30 '10 at 15:55
  • Whoa, just got an upvote 6 months later.. anyway what the OP did really need is to study the concept of "closures", then no more misteries. – Matteo Mosca Dec 27 '11 at 15:52
0

This is because of how closures work in JavaScript. Each of the 5 functions you are creating is basically sharing the same i variable. The value of i inside your function is not being evaluated when you are creating the function, but when the click event occurs, by which time the value of i is 5.

There are various techniques for getting around this (when this behavior isn't what you want). One (if you have a simple function, like you do here) is to use the Function constructor instead of a function literal:

$("#some_button" + i).click(new Function("alert("+i+")");
jhurshman
  • 5,861
  • 2
  • 26
  • 16
-1
  (function (some_number) {
    for (i = 1; i <= some_number; i++) {
      $("#some_button" + i).click(function() {
        alert(i);
      });
    }
  })(some_number);

Wrap the function outside because for speed and the fact i will keep resetting.

RobertPitt
  • 56,863
  • 21
  • 114
  • 161
-1

This is very clever code. So clever it's a question on SO. :) I'd sidestep the question altogether by dumbing the code down, just to have a chance at understanding it (or having a colleague understand it) six months from now. Closures have their place, but in this case I'd avoid them in favour of more understandable code.

Probably, I'd attach the same function to all the buttons, which would get the button from the event, strip "some_button" from the ID, and alert the result. Not nearly as pretty, but I guarantee everyone in the office could follow it at a glance.

Ed Daniel
  • 522
  • 11
  • 22