-1

I'm trying to iterate through an array, like so:

var locs = [
  ['Location 0', 50, 91], 
  ['Location 1', 50, 100]
];

for (i = 0; i < locs.length; i++) {
    $('#' + i).click(function(i) {
        map.panTo(new google.maps.LatLng(locs[i][1], locs[i][2]));
    })

}

But I get the unexpected identifier because of the way loops work in Javascript. I need it to iterate through the locs array.

How do I do this? So frustrated!

I tried the return function thing but it didn't work.

E.G.:

for (i = 0; i < locations.length; i++) {
      marker = new google.maps.Marker({
        position: new google.maps.LatLng(locations[i][1], locations[i][2]),
        map: map
      });

      $('#' + i).click(function(marker, i) {
          return function() {
            map.panTo(new google.maps.LatLng(locations[i][1], locations[i][2]));
          }
      )};
}
AAA
  • 1,962
  • 4
  • 30
  • 55

2 Answers2

1

What you appear to be doing is indexing your clickable elements numerically where it would probably be better to give them all a class and maybe give them some data- attributes to indicate any further information rather than numerical id's. This has a number of benefits, first once you use an Id you should never use it again. #1, #2 etc. would need to become something else if you wanted to use the same technique again in the same DOM structure. As well as this, what would happen if you pushed a new value to your locs array in the future and wanted to then bind an elements click event how would you achieve it?

Maybe a better type of solution would involve using delegated events and avoiding indexing by Id. So to index you can use the DOM structure itself, or apply a data- attribute.

example element

<a class="map-location" data-index="1">Location 1</a>

JS

var locs = [
    ['Location 0', 50, 91], 
    ['Location 1', 50, 100]
];

$(document).on('.map-location', 'click', function(event) {
    var self = $(this),
        index = self.data('index'), // Index by `data-index`
        selfIndex = self.parent().index(self); // Or index by DOM location

    map.panTo(new google.maps.LatLng(locs[index][1], locs[index][2]));
});

No need to side step closures or using expensive $.each calls, also new locations can be pushed to the locs array at any time, as long as an element is created with the correct class and index to click on, it will have a bound event to pan to the marker.

David Barker
  • 14,484
  • 3
  • 48
  • 77
  • *No need to side step closures or using expensive $.each calls* Yes, storing data like that avoids those pesky expensive function calls.... :) – alex Jul 13 '13 at 12:34
0

But I get the unexpected identifier because of the way loops work in Javascript

It has nothing to do with the way loops work.

I tried the return function thing but it didn't work.

Returning a function won't do anything. The return value must be a boolean from an event handler's function.

You've got the closures problem.

You're in luck 'cos you're using jQuery, and $.each() will side-step the closure problem.

$.each(locs, function(i) {
    $('#' + i).click(function(index) {
        map.panTo(new google.maps.LatLng(locs[i][1], locs[i][2]));
    });
});

What you should really understand is that inner function closed over i, and it holds a reference to it.

By the time that click handler is called, it will look up i in the scope chain and find the scope where it was defined. When it does, that for loop will have incremented it beyond a subscriptable array element.

alex
  • 479,566
  • 201
  • 878
  • 984
  • Ya, don't read it enough :) – A. Wolff Jul 13 '13 at 09:33
  • Yes, I tried the return function thing like the Google example because of the closures, but I must have done it wrong. I added my dysfunctional code with closures in mind. – AAA Jul 13 '13 at 09:34
  • @AAA Unfortunately, the code you tried won't do it. You can use an IIFE or `with() {}`, or just use jQuery's iterator. – alex Jul 13 '13 at 09:37
  • Thanks, works well. Thanks for the vid on closures. I'll watch it. – AAA Jul 13 '13 at 09:38
  • Now for some reason, the code will update the infowindows but it won't go to the new location. It goes to the last location in the list. – AAA Jul 13 '13 at 09:42
  • Was going to accept your answer, but did not realize the closures video was not actually a closures video. Rude. – AAA Jul 13 '13 at 10:25
  • @AAA Rude because your assumption proved to be incorrect? – alex Jul 13 '13 at 12:33
  • @alex Rude because the video you initially linked was some sarcastic thing that had to do with magnets. Also, a mod edited out your answer. – AAA Jul 13 '13 at 14:46
  • @AAA It was a meme that I decided to post for some stupid reason. Another contributor censored it, I edited it myself out of my answer. – alex Jul 14 '13 at 00:11