What am I doing wrong on this simple Closure

Question

Hi all, it maybe sleep deprivation but I cannot understand why this isn't working. I want the onclick to return the value of i from a for loop that created the element and applied the event handler. Have put it in a closure and it is still turning the max number of the iterator.

window.onload = function(){
  var arbitrary_amount = 100;
  var the_list = document.getElementsByTagName('ul')[0];

  for(var i = 0; i < arbitrary_amount; i++){
    var natural_index = i + 1;
    var list_item = document.createElement('li');
    var inner_link = document.createElement('a');
    inner_link.setAttribute('href', '#');
    inner_link.innerHTML = "Link "+natural_index;

    inner_link.onclick = function(){
      return function(link_num){
        alert('You clicked link '+link_num);
      }(i);
    };

    list_item.appendChild(inner_link);
    the_list.appendChild(list_item);
  }


};

Answer 1

you are using the closure in the wrong way...i can't give you a guru type answer as to what was actually happening but here is a working (didn't test it) closure:

inner_link.onclick = (function(link_num){
   return function(){
       alert('You clicked link '+link_num);
   };
})(i);

Answer 2

You can try with:

window.onload = function(){
  var arbitrary_amount = 100;
  var the_list = document.getElementsByTagName('ul')[0];

  for(var i = 0; i < arbitrary_amount; i++){
    (function(){var natural_index = i + 1;
    var list_item = document.createElement('li');
    var inner_link = document.createElement('a');
    inner_link.setAttribute('href', '#');
    inner_link.innerHTML = "Link "+natural_index;

    inner_link.onclick = function(){
      return function(link_num){
        alert('You clicked link '+link_num);
      }(i);
    };

    list_item.appendChild(inner_link);
    the_list.appendChild(list_item);})();
  }


};

Answer 3

Your code works correctly for me in Firefox 3.5.5 and Chrome 4.0.249.64.

Answer 4

It's because each closure you create for the onclick handler shares the same environment. When the onclick callback function is executed, each one refers to the last value of i.

There are already various solutions out, so I won't repeat it here, but the idea is to create additional closures that do not share the same environment.

It's a common mistake. Checkout this MDC article for more insights.