Adding Google Maps markers - Javascript 'IF' does not work in 'FOR' loop

Question

I would like to add different coloured markers to a Google Map; if the address is a rental house ("Y"), and add red markers if the address belongs to a private advertiser ("N"). I thought it could be easily done by an IF condition, but somehow it does not work and I can not figure out the solution. The if (usertypes[x]=="Y") {} is not recognised so it automatically jumps to else {}, that's why the result is always blue now.

The code:

var addresses = ['address1', 'address2', 'address3', 'address4', 'address5'];
var usertypes = ['Y', 'N', 'Y', 'Y', 'N'];
var bounds = new google.maps.LatLngBounds();

// the loop:
for (var x = 0; x < addresses.length; x++) {
  $.getJSON('http://maps.googleapis.com/maps/api/geocode/json?address=' + addresses[x] + '&sensor=false', null, function(data) {
    var p = data.results[0].geometry.location
    var latlng = new google.maps.LatLng(p.lat, p.lng);

    // set red marker if the advertiser is a private user:
    if (usertypes[x] == "Y") {
      new google.maps.Marker({
        position: latlng,
        map: map,
        icon: 'http://maps.google.com/mapfiles/ms/icons/red-dot.png',
        title: 'item position'
      });

      bounds.extend(latlng);

      // set blue marker if it is a rental house:
    } else {
      new google.maps.Marker({
        position: latlng,
        map: map,
        icon: 'http://maps.google.com/mapfiles/ms/icons/blue-dot.png',
        title: 'item position'
      });
      
      bounds.extend(latlng);
    }
  });
}

map.fitBounds(bounds);

Thanks for your help :)

Have you actually looked at `usertypes[x]`? Can you confirm that it is ever that exact value of `"Y"`? I'm pretty certain `if` statements didn't just suddenly stop working correctly. — Mike Cluck, Aug 19 '16 at 21:52
Then it likely has one or more spaces around it. Also, `x` is not going to be the correct value when that `$.getJSON` is done. [Read this question to find out why.](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) — Mike Cluck, Aug 19 '16 at 21:55
@MikeC there is no space around, and the x value looks correct. Otherwise addresses would not appear on the map either — Norbert Biró, Aug 19 '16 at 22:00
I'm sure `x` is fine sometimes but I promise you that you're using the *last* `x` value by the time your callback runs. So if you have multiple addresses, it's not going to work correctly. I'm also 100% positive that if `usertypes[x]` is `"Y"` then [your `if` statement will work.](https://jsfiddle.net/L9m4b1vL/1/) So you need to really, really check that you're getting the data you're expecting. If they are the same value, the `if` statement **will** work. — Mike Cluck, Aug 19 '16 at 22:03
@MikeC you were totally right.. In this case I would like to know how to solve this issue.. Do you have any suggestion maybe? — Norbert Biró, Aug 19 '16 at 22:32

Ilmari Karonen · Accepted Answer · 2016-08-19T22:32:30.670

Your problem is that your callbacks execute asynchronously, only after the Ajax requests have completed. Since this is also after your loop has finished, the value of the variable x at that point is always equal to addresses.length.

Here's a simple snippet, stripped of all the Google-specific stuff, that demonstrates the problem:

var addresses = ['address1', 'address2', 'address3', 'address4', 'address5'];
var usertypes = ['Y', 'N', 'Y', 'Y', 'N'];

for (var x = 0; x < addresses.length; x++) {
  setTimeout(function() {
    if (usertypes[x] == "Y") {
      console.log('red', x);
    } else {
      console.log('blue', x);
    }
  }, 1);
}

When run, this snippet logs blue 5 five times to the console. If you remove the setTimeout() and just call the function synchronously, however, you get the expected output (red 0, blue 1, red 2, red 3, blue 4) instead.

Of course, since you're using Ajax, you can't really do that. Instead, you need to somehow "freeze" the value of x in your callbacks. One way to do that is to pass it as an argument to a (synchronous) wrapper function:

var addresses = ['address1', 'address2', 'address3', 'address4', 'address5'];
var usertypes = ['Y', 'N', 'Y', 'Y', 'N'];

for (var x = 0; x < addresses.length; x++) {
  (function (x) {
    setTimeout(function() {
      if (usertypes[x] == "Y") {
        console.log('red', x);
      } else {
        console.log('blue', x);
      }
    }, 1);
  })(x);
}

Every time the wrapper function is called in the loop, a new variable named x is created inside it (masking the outer loop counter variable of the same name) to store the value passed into the function as its argument. But since this value is simply the current value of the outer x, the net effect is to lock the value of x inside the wrapper to the value the outer x had when the wrapper was called.

Another option, in modern JavaScript, would be to rewrite the loop using .forEach():

var addresses = ['address1', 'address2', 'address3', 'address4', 'address5'];
var usertypes = ['Y', 'N', 'Y', 'Y', 'N'];

addresses.forEach(function (address, x) {
  setTimeout(function() {
    if (usertypes[x] == "Y") {
      console.log('red', x);
    } else {
      console.log('blue', x);
    }
  }, 1);
});

Here, the callback function for .forEach effectively acts as the wrapper function, with each invocation of it having its own separate x. As a bonus, the code also looks quite a bit tidier.

this answer is pretty promising. However, I am not really into this topic yet and I don't know where and/or how should I insert these lines into my code.. — Norbert Biró, Aug 19 '16 at 23:01
The `setTimeout()` in my snippets is a stand-in for your `$.getJSON()`. (Both take a callback function and schedule it for asynchronous execution later.) You should wrap that entire callback inside the wrapper function. — Ilmari Karonen, Aug 19 '16 at 23:23

Adding Google Maps markers - Javascript 'IF' does not work in 'FOR' loop

1 Answers1