1

I have the following for loop:

for (loc in locations){
    var newLoc = locations[loc].split(", ")
    var uniquevar = new google.maps.Marker({
    position: new google.maps.LatLng(newLoc[0], newLoc[1]),
        map: map,
        title: loc
    });
    google.maps.event.addListener(loc, 'click', function() {
        console.log(loc);
    });
};

I't suppose to generate a bunch of map-markers and console.log their name (loc) when you click them. But they all end up console.logging the last item in locations log.

I assume this is because they are all named the same thing

Why is this and what can I do about it?

Cœur
  • 37,241
  • 25
  • 195
  • 267
Himmators
  • 14,278
  • 36
  • 132
  • 223

2 Answers2

4

As sayed above, you need closure:

for (loc in locations){
  (function(loc){  
    var newLoc = locations[loc].split(", ")
    var uniquevar = new google.maps.Marker({
      position: new google.maps.LatLng(newLoc[0], newLoc[1]),
      map: map,
     title: loc
    });

    google.maps.event.addListener(loc, 'click', function() {
      console.log(loc);
    });
  })(loc);
};
Nayjest
  • 1,089
  • 8
  • 14
  • This is a nice terse way of creating a closure in a loop - much neater than creating a separate function to return a closure and calling it on click as I had suggested! – chrisfrancis27 Jun 13 '12 at 16:09
  • 1
    Also here, that's not correct. The event handlers *are already closures* and that is what is causing the problem. You have to call a function to create a new scope. That's what you do with `(function(loc){...})(loc);`. Of course this function *is a closure too*, but that is not the important point. It solves the problem because you are *calling this function immediately* and pass `loc` as argument. – Felix Kling Jun 13 '12 at 16:17
  • 2
    OK, so really the important point is taking the value of `loc` at the time each loop iteration runs, as opposed to only evaluating it in the click handler, at which point the loop has already run and `loc` only points to the last enumerable `locations` property. – chrisfrancis27 Jun 13 '12 at 16:21
  • THat last comment really made sense to me! – Himmators Jun 13 '12 at 20:34
  • Except why the loop has already run look at the click handler... – Himmators Jun 13 '12 at 20:34
  • And what does the last (loc) do, the one after the function? – Himmators Jun 13 '12 at 20:36
0

You need to create a closure. By creating a closure, you effectively 'bake' the state of the internal function at the time of execution, which is NOT the same time as the 'click' handler is fired.

EDIT: Removed terrible code - use Nayjest's much cleaner solution above!

Community
  • 1
  • 1
chrisfrancis27
  • 4,516
  • 1
  • 24
  • 32
  • You have errors in this code: logLocation must be called with loc argument instead of immediate execution without arguments and passing it later to google.maps.event.addListener – Nayjest Jun 13 '12 at 16:08
  • Yeah, totally untested madness :) Will fix it up – chrisfrancis27 Jun 13 '12 at 16:09
  • That's not correct. The event handlers are already closures. Actually you don't want to use closures, you want to call a function and pass the value as argument so that it creates a new scope. You still have the closure and that's why it does not work. Your code should be: `google.maps.event.addListener(loc, 'click', logLocation(loc))`. – Felix Kling Jun 13 '12 at 16:12
  • Thanks man - yeah the code was totally ballsed-up. But interestingly you say to call a function to create a new scope, rather than use a closure, but isn't that the same thing? I see closures as a way of baking a scope at execution time. Or am I misunderstanding? – chrisfrancis27 Jun 13 '12 at 16:15
  • 2
    Any function that you define anywhere is a closure (in JavaScript), since it has access to variables defined in higher scopes (like the event handler has access to `loc` from the `for` loop). And that's the problem if you define functions in a loop. But by *calling* a function, you create a new execution context and a new scope. Only then the value you pass to the function is "captured". See my comment to the other answer and my the question I linked to. – Felix Kling Jun 13 '12 at 16:19
  • 1
    Other languages might handle it differently, but that's how it is in JavaScript. Inside a closure, the value of a (higher-scope) variable is evaluated at the time the function is executed, not when it is defined. – Felix Kling Jun 13 '12 at 16:24
  • Thanks Felix, so I think I understand it - there is no block scope so the `loc` variable exists in the outer function (wherever the loop is declared) - therefore of course there is only one `loc` var, getting repeatedly overwritten throughout the loop iterations, eventually ending up at the final value. By passing it into a function within the loop iteration, you take a copy of the value at that time... – chrisfrancis27 Jun 13 '12 at 16:26
  • Exactly :) (also your comment to the other answer is correct). You don't have to pass it as argument actually (but it looks cleaner). Important is that you are executing a function immediately inside the loop and therefore the variable is evaluated at that time. Without that, the variable is evaluated when the event handler is executed which is much later, after the `for` loop finished. – Felix Kling Jun 13 '12 at 16:27