1
function loadGroups() {
  new Ajax.Request("https://www.xyz.com/groups.json", {
    method: 'get',
    onSuccess : function(transport) {

      var response = JSON.parse(transport.responseText);

      for (var i=0; i<response.length; i++) {
          var hostname = response[i];
          var tr = new Element("tr", { id: hostname });
          var link = new Element("a");
          link.setAttribute("href", id=hostname);
          link.innerHTML = hostname;

          # PROBLEM
          # This onClick always sends response[-1] instead of response[i]
          link.onclick = function() { loadHosts(hostname); return false; };


          var td1 = new Element("td", { className: hostname });
          link.update(hostname);
          td1.appendChild(link);
          tr.appendChild(td1);
          table.appendChild(tr);

      }
    }
  });
}

response = ['john', 'carl', 'jia', 'alex']

My goal is in this case 4 links should be displayed on the page and whenever a link is clicked the corresponding value should be passed to function - loadHosts. But the problem is always the last value gets sent as parameter to loadHosts function. The display of links is fine it is just the click on these links passing always the last element of array. I also tried loadHosts(link.href) , loadHosts(link.id)

jko
  • 2,068
  • 13
  • 25
user1579557
  • 373
  • 1
  • 6
  • 15
  • 1
    This is very hard to tackle the way you present it. Before we know anything, you post a wall of code that contains **a lot** of unnecessary information. Try to reduce your question to its minimal relevant form, so that people will read it :) – salezica Dec 19 '13 at 20:41
  • @uʍop ǝpısdn : done. :) – user1579557 Dec 19 '13 at 20:47
  • @floww: yes the code works. I tried to strip down not so important parts. – user1579557 Dec 19 '13 at 20:51
  • possible duplicate of [Javascript closure inside loops - simple practical example](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) – Bergi Dec 19 '13 at 21:06

2 Answers2

2

Your problem is with the way closures work, you have a click handler inside a for loop that is using a variable defined inside the for loop. By the time the click handler runs, your variable has a different value than when you attached the handler because the loop has finished and it now has a different value.

Try something like this:

for (var i=0; i<response.length; i++) {
    var hostname = response[i];
    //...
    link.onclick = (function(hn) {
       return function() { loadHosts(hn); return false; };
    })(hostname);
}

This uses a immediately invoked function expression (IIFE) to copy the value of hostname to the a new variable hn during the loop. Now when the handler is executed, it will be bound to the correct value.

Here's a blog post that explains the problem (and solution).

Also note that for loops don't define a variable scope. Variables defined in your for loop belong to the enclosing function scope. So hostname exists outside of your for loop which is why it holds the value of the last cycle through the loop in your click handler rather than the value when you attached the handler. It also explains why link.href doesn't work either. The variable link is attached to the enclosing function scope too rather than limited to the loop.

Matt Burland
  • 44,552
  • 18
  • 99
  • 171
1

you should really go with the solution of @MattBurland but if you like, you could also tie the variable to the object itself.

link.hostname = response[i];
link.onclick = function() { loadHosts(this.hostname); return false; };

see simple example here: http://jsfiddle.net/xeyqs/

jko
  • 2,068
  • 13
  • 25