Hacker News Reader

Comment by krmmalik | Hacker News Reader

Is it poor etiquette to state what i think the answer is?

Please do.

It won't alert quite what you want - when you click on each element in the list, it will alert "4".

This is because the function assigned to the click event of each li is bound to the "i" variable used in the loop by a closure. Variable i is incremented to 4 before the loop ends, so that is what each will print.

You could use make the function used in the click handler take a parameter, then use partial function application (or currying) to fix the value of the parameter.

The function in the click handler does take a parameter 'e' (...or whatever you call it) which is an Event object. But I think you're on to something by using currying.

Well yes, the function is called with a MouseEvent as one of the arguments, so you could use the "arguments" object to access it.

What I was thinking of, was something along the lines of:

  // From http://ejohn.org/blog/partial-functions-in-javascript/
  Function.prototype.curry = function() {
    var fn = this, args = Array.prototype.slice.call(arguments);
    return function() {
      return fn.apply(this, args.concat(
        Array.prototype.slice.call(arguments)));
    };
  };

  els = document.getElementsByTagName('li');
  for(i=0; i < els.length; i++){
    els[i].addEventListener('click', function(x){
        alert(x);
    }.curry(i), false);
  }

Wrap the line of code inside the loop inside another function, which will have the effect of binding the iterator i to the desired value before the event handler anonymous function is created.

this is what I usually tell people to do:

  var els = document.getElementsByTagName('li');//use var unless you really really really meant global
  for(var i=0; i < els.length; i++){
    (function(i) {//creates new scope for each iteration
      els[i].addEventListener('click', function(){alert(i);}, false);
    })(i);
  }

i agree with the choice of code, but I have a different style for laying out the code.

i dislike the declaration and immediate execution of an anonymous function. i much prefer to give it a name, move it out of the loop itself and then call it within the loop. it makes reading the code more top-to-bottom.

with this approach, when I get to the anon func declaration, i have to scan down looking for when it is invoked. while indentation helps, i then have to backtrack up to the beginning of the func declaration.. and hopefully i haven't lost track of what the execution context for the anon func is and so forth...

I don't generally care too much to talk about style when the problem at hand is that someone has been banging their head against the desk for the past two hours and they just want it to work (and, in my experience, this usually is the case, for some reason)

But yeah, if we're gonna be super nitpicky, yeah sure, improve the for loop also, while we're at it.

  for (var i = -1; els[++i];) {//given the code in the loop body, this is shorter and faster

Wouldn't var i have to be set to -1:

  for (var i = -1; els[++i];)  ?

Otherwise, the alert won't trigger on the first li element.

Hah, good catch. Was thinking of i++ for some reason. Fixed.

Does this work (from a non-JS person -- I like reading JS books, but don't code in it at all). This is certainly less elegant, but seems easier to follow:

  var els = document.getElementsByTagName('li');

  for(i=0; i < els.length; i++){
    var j = i;
    els[j].addEventListener('click', function(){alert(j);}, false);
  }

That doesn't work. Variables in javascript are not block scoped but function scoped.

Thanks. Good to know.

No, it doesn't because j is in the same scope as i.

Your confusion is because of a javascript quirk

  for (...) {
    var foo = bar
  }

is actually the same as

  var foo
  for (...) {
    foo = bar
  }

Notice how you only ever have one foo

What you need is:

  for (...) {
    (function(bar) {
       var foo = bar
    })(bar)
  }

which create a new foo every time you loop

Reply on news.ycombinator.com