jQuery click binding function in Javascript loop using last parameter

Question

This is probably me being silly as per usual... but this is frustrating me.

I have a foreach loop going through a multidimentional array, and for each item I'm adding a marker to a custom leafletjs map. Everything's working fine, except I have a custom click event on the marker (the fact that it's leafletjs should be irrelevant), and it's only ever using the last passed parameter.

Here's the code:

var markerArray = [
    ["aspiration", -55.63342652633955, -54.1953125, "aspiration-high-school", [19, 29], [38, 57], [75, 113], [150, 226], [299, 451], [597, 901]],
    ["bird", 12.89745, 69.99818, "bird", [22, 20], [44, 40], [87, 79], [173, 158], [345, 315], [689, 629]],
    ["camera", -22.908054128088575, -29.15421875, "camera", [15, 22], [29, 44], [57, 88], [113, 176], [225, 351], [450, 702]],
    ["chef", -46.6909603909255, 106.193125, "chef", [12, 54], [23, 108], [45, 215], [90, 430], [179, 859], [358, 1718]],
    ["computer", -57.37383096593114, 92.71484375, "computer", [14, 18], [28, 36], [56, 72], [112, 144], [223, 288], [445, 576]],
    ["film", -25.91792293614603, -43.69140625, "film", [13, 19], [25, 38], [49, 75], [98, 149], [196, 298], [392, 595]],
    ["gotthejob", -0.490228926463384, -61.02343749999999, "got-the-job", [40, 10], [80, 19], [160, 38], [320, 75], [639, 150], [1278, 300]],
    ["headmaster", -8.5, -60.57812500000001, "headmaster", [40, 44], [80, 87], [160, 173], [320, 346], [639, 692], [1278, 1383]],
    ["letter", -78.77138592818217, -57.15, "letter", [13, 11], [26, 21], [51, 42], [101, 83], [202, 166], [404, 331]],
    ["newspaper", -67.64766505841037, 92.6578125, "news-paper", [21, 15], [41, 30], [82, 59], [163, 117], [325, 234], [649, 467]],
    ["openday", -69.69785394109224, 24.3578125, "open-day", [15, 17], [29, 34], [57, 67], [113, 133], [226, 266], [452, 531]],
    ["prospectus", -11.885147283424319, -136.39453125, "prospectus", [37, 57], [74, 113], [148, 226], [295, 452], [590, 904], [1180, 1808]],
    ["ruler", -73.92669969306126, 4.94609375, "ruler", [12, 11], [24, 22], [48, 43], [96, 85], [192, 169], [384, 337]],
    ["schoollogo", 9.64906182688142, -108.50859375, "school-logo", [21, 27], [41, 53], [81, 106], [161, 211], [321, 421], [641, 841]],
    ["schoolplay", -3.013667927566642, -159.54921875, "school-play", [18, 43], [36, 86], [71, 172], [142, 344], [283, 687], [566, 1373]],
    ["sun", 14.98818922264095, 20.26953125, "sun", [40, 18], [79, 36], [158, 72], [316, 143], [631, 285], [1262, 570]],
    ["train", -81.93133285369295, -141.99609375, "train", [42, 30], [83, 60], [165, 119], [330, 237], [659, 473], [1317, 946]]
];
var zoom0MarkersArr = new Array();
var zoom1MarkersArr = new Array();
var zoom2MarkersArr = new Array();
var zoom3MarkersArr = new Array();
var zoom4MarkersArr = new Array();
var zoom5MarkersArr = new Array();
for (var arrayIndex = 0; arrayIndex < markerArray.length; arrayIndex++) {
    var i = 0;
    while (i <= 5) {
        var thisMarkerTypeStr = markerArray[arrayIndex][0];        
        var thisMarkerLat = markerArray[arrayIndex][1];
        var thisMarkerLng = markerArray[arrayIndex][2];
        var thisMarkerImage = markerArray[arrayIndex][3];
        var thisMarkerImageSizeArr = markerArray[arrayIndex][i + 4];
        var thisIcon = L.icon({
            iconUrl: '../images/map/elements/' + i + '/' + thisMarkerImage + '.png',
            iconSize: thisMarkerImageSizeArr,
            className: thisMarkerImage + ' leaflet-zoom-hide'
        });
        var thisMarker = L.marker([thisMarkerLat, thisMarkerLng], {
            icon: thisIcon
        }).on("click", function () { ShowDetails(thisMarkerTypeStr); });
        window['zoom' + i + 'MarkersArr'].push(thisMarker);
        i++;
    }
}
var zoom0Markers = L.layerGroup(zoom0MarkersArr);
var zoom1Markers = L.layerGroup(zoom1MarkersArr);
var zoom2Markers = L.layerGroup(zoom2MarkersArr);
var zoom3Markers = L.layerGroup(zoom3MarkersArr);
var zoom4Markers = L.layerGroup(zoom4MarkersArr);
var zoom5Markers = L.layerGroup(zoom5MarkersArr);

The function ShowDetails is currently just logging the name of the element clicked, which is always train.

The variable being passed to the ShowDetails function, is a locally scoped one, so not sure why it's not working for me.

Any help/obvious suggestions would be appreciated.

handler is executed once event is fired so thisMarkerTypeStr is the last value. BTW, looks like thisMarkerTypeStr is global — A. Wolff
– A. Wolff, Commented Dec 10, 2013 at 14:08
Hmm.. So actually, without doing this outside of a loop, I'm not going to be able to do this? I do have it working out of a loop, but there's an awful lot more code :-(. I guess if that's the way it has to be then that's that, was just hoping I could make it a little more elegant. Thanks for the pointer, I feared that may be the reason. — Mark Johnson
– Mark Johnson, Commented Dec 10, 2013 at 14:14
Sorry, thought my edit would show up, I did add a comment to the changes.. but yes, I did change the scope of the variables to what is in my code now, I had been playing with the code too much and pasted the wrong version. — Mark Johnson
– Mark Johnson, Commented Dec 10, 2013 at 14:17
I see now, thank you! I don't do nearly enough javascript at the moment, I'm sure things like this will become obvious one day! — Mark Johnson
– Mark Johnson, Commented Dec 10, 2013 at 14:19

Renato Zannon · Accepted Answer · 2013-12-10 14:32:54Z

Javascript var variables are function-scoped, instead of block-scoped. That means that your thisMarkerTypeStr is the same throughout the whole function. The behavior can be visualized using this classic setTimeout example:

for(var i = 0; i < 5; i++) {
  setTimeout(function() { console.log(i) }, 100);
}
// 100ms afterwards, the number 5 is printed 5 times

A possible workaround, without having to change much code, is to create a closure inside the loop, that captures the variable:

for(var i = 0; i < 5; i++) {
  setTimeout((function(localI) {
    return function() { console.log(localI) };
  }(i)), 100);
}

Another, more (IMHO) syntatically pleasant way, is to create a maker function outside of the loop:

for(var i = 0; i < 5; i++) {
  setTimeout(logMaker(i), 100);
}

function logMaker(i) {
  return function() { console.log(i); }
}

Applying the last one to your situation:

var thisMarker = L.marker([thisMarkerLat, thisMarkerLng], {
    icon: thisIcon
});

thisMarker.on("click", ShowDetailsMaker(thisMarkerTypeStr));

// ....

function showDetailsMaker(marker) {
    return function() {
        ShowDetails(marker);
    }
}

A. Wolff · Accepted Answer · 2013-12-10 14:23:41Z

1

Try using with statement if you dislike more classical closure:

var thisMarker = L.marker([thisMarkerLat, thisMarkerLng], {
    icon: thisIcon
});
with({
    thisMarkerTypeStr: thisMarkerTypeStr
}) {
    thisMarker.on("click", function () {
        ShowDetails(thisMarkerTypeStr);
    });
}

answered Dec 10, 2013 at 14:23

A. Wolff

74.4k9 gold badges98 silver badges158 bronze badges

2 Comments

Mark Johnson Over a year ago

Thank you again! That works perfectly. As yet I've not needed to use the with statement, so I'll have to do some looking up. Much appreciated.

Renato Zannon Over a year ago

It is important to note that using the with statement is not recommended. Not that you can't use it, but you should be aware of the potential issues :)

Collectives™ on Stack Overflow

jQuery click binding function in Javascript loop using last parameter

2 Answers 2

Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related