Anonymous function in for loop

Question

I am using Bing Maps to implement putting multiple pins on a map. Whenever a pin is pressed I have an infobox popup and within the info box i have an edit button. When the edit button is pressed i want it to display the title associated for the pin (testing purposes). However, whenever I add a handler in my for loop for each pin only the last handler is used... For example if I add three pins with the titles: [hello, foo, bar], bar is displayed no matter what pin I click on... Here is what I am doing:

for ( var pos = 0; pos < locationsSize; pos++) {

            var locationFromIndex = locations[pos];
            var bingLocation = new Microsoft.Maps.Location(
                    locationFromIndex.latitude, locationFromIndex.longitude);

            // Create/add the pin
            var pin = new Microsoft.Maps.Pushpin(bingLocation, {
                width : 25,
                height : 39,
                anchor : mAnchor
            });
            pins.push(pin);

            // Create/add the pin info box
            var pinInfobox = new Microsoft.Maps.Infobox(pin.getLocation(), {
                title : locationFromIndex.type,
                visible : false,
                height : 75,
                zIndex : i,
                width : 150,
                offset : mOffset,
            })
            pinInfobox.setOptions({
                actions : [ {
                    label : "Edit",
                    eventHandler : function(mouseEvent) {
                        alert(pinInfobox.getTitle()); // Only the last eventHandler added is being used...
                    }
                } ]
            });
            map.entities.push(pinInfobox);
        }

Amusingly, I [just answered another version of this question](http://stackoverflow.com/questions/16794707/javascript-generate-dinamically-handler/16794762#16794762). See that, or of course [the one that's a duplicate of](http://stackoverflow.com/questions/4091765/assign-click-handlers-in-for-loop). But I offer more options. Your `pin` is `i` in [that answer I just linked](http://stackoverflow.com/questions/16794707/javascript-generate-dinamically-handler/16794762#16794762). — T.J. Crowder, May 28 '13 at 17:46

score 1 · Accepted Answer · answered May 28 '13 at 17:47

1

The simplest solution to your problem is a closure:

for ( var pos = 0; pos < locationsSize; pos++) {
  (function(locationFromIndex) {
        var bingLocation = new Microsoft.Maps.Location(
                locationFromIndex.latitude, locationFromIndex.longitude);

        // Create/add the pin
        var pin = new Microsoft.Maps.Pushpin(bingLocation, {
            width : 25,
            height : 39,
            anchor : mAnchor
        });
        pins.push(pin);

        // Create/add the pin info box
        var pinInfobox = new Microsoft.Maps.Infobox(pin.getLocation(), {
            title : locationFromIndex.type,
            visible : false,
            height : 75,
            zIndex : i,
            width : 150,
            offset : mOffset,
        })
        pinInfobox.setOptions({
            actions : [ {
                label : "Edit",
                eventHandler : function(mouseEvent) {
                    alert(inInfobox.getTitle()); // Only the last eventHandler added is being used...
                }
            } ]
        });
        map.entities.push(pinInfobox);
    }
 })(locations[pos]);

The closure closes over its containing scope, but will have a specific reference to your locations[pos] in each call to it. This allows you to not have the loop issue.

answered May 28 '13 at 17:47

Sébastien Renauld

19,203
2
46
66

This creates unnecessary functions (objective) and is hard to read (subjective). Still, the objective issue is unlikely to cause anyone any real trouble... – T.J. Crowder May 28 '13 at 17:49
*"The simplest solution to your problem is a closure"* The OP's function is **already** a closure. That's why he/she is having trouble in the first palce. – T.J. Crowder May 28 '13 at 17:50
@T.J.Crowder: All I did was to provide a way to scope `pin` in order for his/her events to actually bind to the correct pin instead of always referencing the last one. Also, the OP's declaration of `pin` is **NOT** in one. – Sébastien Renauld May 28 '13 at 17:53
@T.J.Crowder: Finally, while I am open to criticism, all I have done is to fix the problem. Refactoring his/her code is none of my business - I merely provided a reason for the problem she has (`pin` is hoisted) and a solution. – Sébastien Renauld May 28 '13 at 17:54
@ Sébastien: Yeah, absolutely, and good on you for that. And all I did was say why I find that common solution less than ideal, for both objective and subjective reasons. I made a *point* of highlighting the fact that the objective reason is probably not all that important, and that the subjective reason is...subjective. :-) – T.J. Crowder May 28 '13 at 17:59
Thanks guys! I will try it out now. Can I get a brief explanation why this is happening. I am new to javascript... – thunderousNinja May 28 '13 at 18:01
1

@unknown MDN has a good [explanation on the issue](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Closures#Creating_closures_in_loops.3A_A_common_mistake). – Mattias Buelens May 28 '13 at 18:09
Good read @Mattias. Thanks! – thunderousNinja May 28 '13 at 18:16

score 1 · Answer 2 · answered May 28 '13 at 18:06

1

pinInfobox.setOptions({
    actions: [{
        label: "Edit",
        eventHandler: (function (infoBox) {
            return (function (mouseEvent) {
                alert(infoBox.getTitle());
            })
        })(pinInfobox)
    }]
});

answered May 28 '13 at 18:06

Diode

24,570
8
40
51

Anonymous function in for loop

2 Answers2