Having Trouble With Closure In Javascript

Question

I'm trying to pass an array into a function and then use the information in that array to initialize a google map. However, when I click on a marker on the map, an error is produced which says:

Unable to get value of the property 'popupHtml': object is null or undefined

The reason for creating the function is so that the javascript code can be moved to a seperate .js file, thus becoming seperate from the html file. Is there anyway this problem can be corrected? Here's all my code (I put a comment in to mark where the error is occuring...):

<!DOCTYPE html>

<html>
<head>    
    <script type="text/javascript" src="http://maps.googleapis.com/maps/api/js?sensor=false"></script>
    <script type="text/javascript">
        window.onload = function ()
        {
            function loadMap(markers)
            {
                var options =
                {
                    center: new google.maps.LatLng(40.775813, -73.970786),
                    zoom: 17,
                    mapTypeId: google.maps.MapTypeId.ROADMAP
                };
                var map = new google.maps.Map(document.getElementById('map'), options);

                var points = new Array();
                for (var i = 0; i < markers.length; i++)
                    points.push(new google.maps.LatLng(markers[i].lat, markers[i].lon));
                var infoWindow = new google.maps.InfoWindow();
                for (var i = 0; i < markers.length; i++)
                {
                    var googleMarker = new google.maps.Marker({ position: points[i], map: map, title: markers[i].title });
                    google.maps.event.addListener(
                        googleMarker,
                        'click',
                        function ()
                        {
                            // The following line is where the error is occuring:
                            infoWindow.setContent(markers[i].popupHtml);
                            infoWindow.open(map, googleMarker);
                        });
                }
            };

            loadMap([{
                "lat": "40.776512",
                "lon": "-73.970293",
                "popupHtml": "\u003cdiv\u003eHello world - from marker 1!\u003c/div\u003e",
                "title": "Marker 1!"
            },
            {
                "lat": "40.774659",
                "lon": "-73.971548",
                "popupHtml": "\u003cdiv\u003eHello world - from marker 2!\u003c/div\u003e",
                "title": "Marker 2!"
            }]);
        };
    </script>
</head>
<body>
    <div id="map" style="width: 680px; height: 400px;"></div>
</body>
</html>

T.J. Crowder · Accepted Answer · 2012-05-01 09:43:39Z

3

The problem is that the function you're passing into addListener has enduring access to the i variable, not a copy of its value as of when the function was created. So all of the copies of the function see i as of when they're called, which is presumably past the end of the array. The same goes for googleMarker; they'll all see the last value it had in the loop rather than the current value.

You fix it by using a generator function or (if you can rely on ECMAScript5, or if you use an ES5 shim since bind is something a shim can provide) use Function#bind.

Using bind:

for (var i = 0; i < markers.length; i++)
{
    var googleMarker = new google.maps.Marker({ position: points[i], map: map, title: markers[i].title });
    google.maps.event.addListener(
        googleMarker,
        'click',
        (function (index, thisMarker)
        {
            // The following line is where the error is occuring:
            infoWindow.setContent(markers[index].popupHtml);
            infoWindow.open(map, thisMarker);
        }).bind(undefined, i, googleMarker)
    );
}

bind returns a function that, when called, will call the original function with a given this value and the arguments you give it. So in the above, we call bind passing in the value of i and googleMarker, and it gives us a function that, when called, will call our original with those value as arguments. Then we use the arguments (index and thisMarker) instead of i and googleMarker.

If you can't rely on ES5 features in your target browsers and you don't want to use a shim, you can use a generator function:

for (var i = 0; i < markers.length; i++)
{
    var googleMarker = new google.maps.Marker({ position: points[i], map: map, title: markers[i].title });
    google.maps.event.addListener(
        googleMarker,
        'click',
        makeHandler(i, googleMarker)
    );
}

function makeHandler(index, thisMarker) {
    return function ()
    {
        // The following line is where the error is occuring:
        infoWindow.setContent(markers[index].popupHtml);
        infoWindow.open(map, thisMarker);
    };
}

There we're calling makeHandler, passing in the value of i and googleMarker, and makeHandler returns us a function that closes over those arguments (index and thisMarker) rather than i and googleMarker. Since the makeHandler arguments won't change, our function will see the correct values.

This all has to do with how closures work. More about closures: Closures are not complicated

edited May 1, 2012 at 9:43

answered May 1, 2012 at 8:59

T.J. Crowder

1.1m201 gold badges2k silver badges2k bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Andrew Over a year ago

Thank you T.J. You were absolutely right about it being a problem with closure. I've decided to use your solution of a generator function along with one minor modification - I've added an extra parameter to makeHandler so that I can pass in a value for googleMarker. Apparently, my original script actually had two problems with closure.

T.J. Crowder Over a year ago

@Andrew: Good deal, glad that helped! And yes, I see what you mean, you had to do it for googleMarker too, for the same reason as i. I read right past that bit. Just for future readers, I've fixed the answer. Best,

Simon · Accepted Answer · 2012-05-01 09:02:16Z

0

Your definition of the loadMap function is in the window.onload handler, and so is not available until the the page has loaded, and even then the function is in a different scope.

I think you've got things a little backwards - I swap the definition of the loadMap function with the call to loadMap so that the calls are within the onload event and the definition isn't.

answered May 1, 2012 at 9:02

Simon

38k2 gold badges37 silver badges27 bronze badges

2 Comments

T.J. Crowder Over a year ago

The call to loadMap is also within the onload handler, so there's no problem with scope. The problem is the classic creating-a-function-that-closes-over-a-loop-variable.

Simon Over a year ago

So it is - I answered before I had had my first coffee of the day. Sorry

Tim Rogers · Accepted Answer · 2012-05-01 09:06:51Z

0

As T.J Crowder has pointed out, the value of i changes as the loop progresses, even within the closure. As well as his answer, a simple approach to fix this is to create a variable within the scope of the loop.

for (var i = 0; i < markers.length; i++)
    {
        var marker = markers[i]; // new variable is created for each loop iteration
        var googleMarker = new google.maps.Marker({ position: points[i], map: map, title: marker.title });
        google.maps.event.addListener(
            googleMarker,
            'click',
            function ()
            {
                // The following line is where the error is occuring:
                infoWindow.setContent(marker.popupHtml);
                infoWindow.open(map, googleMarker);
            });
    }

answered May 1, 2012 at 9:06

Tim Rogers

21.8k8 gold badges55 silver badges69 bronze badges

2 Comments

T.J. Crowder Over a year ago

No, this won't work, for the same reason the OP's code doesn't work. The closure closes over the variable marker, and there's only one marker variable (JavaScript doesn't have block scope [yet], so there is no "...the scope of the loop..."). So all of the functions would use the last marker, not the marker that existed when the function was created.

Tim Rogers Over a year ago

Doh! Thanks - I've been telling everyone to do this for ages!

Dmitry Serdiuk · Accepted Answer · 2012-05-01 09:21:08Z

0

You can use simple anonymous function, so here you execute a function and the context of that execution will hold your step value.

for (var i = 0; i < markers.length; i++)
{
    var googleMarker = new google.maps.Marker({ position: points[i], map: map, title: marker.title });

    (function(step){google.maps.event.addListener(
        googleMarker,
        'click',
        function ()
        {
            // The following line is where the error is occuring:
            infoWindow.setContent(markers[step].popupHtml);
            infoWindow.open(map, googleMarker);
        });
    )(i);
}

answered May 1, 2012 at 9:21

Dmitry Serdiuk

4683 silver badges18 bronze badges

Collectives™ on Stack Overflow

Having Trouble With Closure In Javascript

4 Answers 4

2 Comments

2 Comments

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

2 Comments

2 Comments

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related