2

I'm extending my previous question, because I still don't fully understand the concept of javascript closures. Take a quick look at the following code, which will put two markers on a map. (The code is slightly modified from my previous question). 

var map = google.maps.somefunctoinstantiatemap();
var address = new Array();
address[0] = '1 Smith Street';
address[1] = '2 Smith Street';
function onpageload()
{
for(var rownum=0; rownum<address.length; rownum++)
{
    geocoder.geocode({
        'address': address[rownum]
    }, function(results, status) {
        geocodeCallBack(results,status,rownum)
    });
}
}

function geocodeCallBack(results, status, argnum)
{
    var marker = new google.maps.Marker({
        map: map,
        position: results[0].geometry.location,
        title: 'arg: '+argnum+' addr:'+results[0].formatted_address
    });

    google.maps.event.addListener(marker, 'click', function(){
        var infowindow = new google.maps.InfoWindow({
            content: marker.title
        });
        infowindow.open(map, marker);
    });
}

Ok, multiple choice....what is the result when user clicks on both markers?

  1. first marker displays 'arg: 0 addr: 1 Smith Street' and second marker displays 'arg: 1 addr: 2 Smith Street'
  2. first marker displays 'arg: 0 addr: 2 Smith Street' and second marker displays 'arg: 1 addr: 2 Smith Street'
  3. first marker displays 'arg: 1 addr: 1 Smith Street' and second marker displays 'arg: 1 addr: 2 Smith Street'
  4. first marker displays 'arg: 1 addr: 2 Smith Street' and second marker displays 'arg: 1 addr: 2 Smith Street'
  5. first marker displays 'arg: undefined addr: undefined' and second marker display 'arg: undefined addr: undefined'

When I run the code, the answer is 3. But I was expecting the answer to be either 4 or 5. Why is it not 4 and why is it not 5?

Improve this question
3
  • I think that can't be your real code, not least because your for loop up top is looping three, not two, times. Also, you seem to have plucked a map variable out of nowhere (nowhere you show, anyway). Commented Aug 10, 2011 at 18:18
  • This is missing a ton of info. "What is the result" of what? You gave two functions with no calls to either function and you reference Markers without any explanation. If anything this seems like a Google Maps, question, not a closures one. Commented Aug 10, 2011 at 18:21
  • @T.J. Crowder - ok, i fixed up those errors... Commented Aug 10, 2011 at 18:23

2 Answers 2

Reset to default
4

You seem to be expecting the closure to close over something (which it does), but you're not using anything in particular that it closes over in it (like the loop index). The argnum value you get in your geocodeCallBack comes from Google (because you've defined rownum as the third argument to your anonymous function, shadowing the loop counter), it's nothing to do with your loop counter.

A few other points about that code:

Your loop at the top is looping three times, not two. You have it going from 0 to <= results.length. results.length is 2, and so that will loop with rownum values 0, 1, and 2. You mean < results.length. (You've fixed this.)

There's no purpose to your anonymous function here:

geocoder.geocode( {'address': address[rownum]}, function(results, status, rownum) {geocodeCallBack(results,status,rownum)});

...since all it does is pass on the arguments it receives. That can and probably should just be:

geocoder.geocode( {'address': address[rownum]}, geocodeCallBack);

...unless the signature of the function is wrong and you shouldn't be declaring all of those arguments to it.

Edit: Based on your comments below, I think you may want this:

geocoder.geocode( {'address': address[rownum]}, makeCallback(rownum));

function makeCallback(therow) {
    return function(results, status) {
        geocodeCallBack(results, status, therow);
    };
}

...or in your "real code" as you put it, where you're actually using that row number as part of a selector:

geocoder.geocode( {'address': address[rownum]}, makeCallback(rownum));

function makeCallback(therow) {
    return function(results, status) {
        geocodeCallBack(results, status, $('#row-' + therow).val());
    };
}

The makeCallback function creates a function to be the callback. The function it creates closes over the therow argument passed into makeCallback, which never changes, and so you get therow being 0 for the callback you create on the first loop, and therow being 1 for the callback you create on the second loop.

In terms of understanding closures, I've written up this post which I think you may find helpful: Closures are not complicated It goes into the mechanics of how closures work in some detail. And the title is no lie: They're not complicated, people tend to tie themselves up in knots because they think they're complicated, but they're not.

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

10 Comments

in my real code, I have something like this geocoder.geocode( {'address': address[rownum]}, function(results, status) { geocodeCallBack(results,status, $('#row-'+rownum).val())}. The problem is that rownum seems to always be hte value 1.
@John: That value is coming from Google, it's nothing to do with your loop counter. You've declared rownum as the third argument the anonymous function accepts, so within the anonymous function, it's going to have whatever value Google passes to the callback as the third argument. Unless you're intentionally receiving that third argument, don't declare it. Then you won't be shadowing rownum -- and then you'll have the problem you expect, because by the time the function gets called, rownum will be 2, not 0 or 1.
While this was going to be my answer too, I can find no indication that the callback should pass a third argument. The doc says it passes just the first 2. Wonder if this is some undocumented parameter.
@T.J. - bah sorry, there shouldn't be any 3rd arg in the anonymous function. I just corrected the comment.
@John: I've updated the answer with what I think you're looking for.
|
0

geocode() is overriding the value of numrow when you declare it as a function parameter.

for(var rownum=0; rownum<=address.length; rownum++)
{
        geocoder.geocode( {'address': address[rownum]}, function(results, status) {geocodeCallBack(results,status,rownum)});
}

Try this instead, this way geocodeCallBack is sent the rownum that is declared in the forloop

The result should be first marker displays 'arg: 0 addr: 1 Smith Street' and second marker displays 'arg: 1 addr: 2 Smith Street'

Improve this answer

1 Comment

If he does that, geocodeCallback will always see 2 as its third argument. Definitely not what the OP wants.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.