1

Possible Duplicate:
How to get around the jslint error ‘Don’t make functions within a loop.’

Fair warning, I'm VERY much a beginner. I'm working on a google maps api v3 project (http://jsbin.com/ofepet/9/edit) and I have a "Don't make functions within a loop" warning on JSBin. I want to fix it but I'm using code that I got elsewhere so I'm struggling to understand exactly what's going on -- particularly with the last 7 lines.

In short, I don't understand the code well enough to take the function out of the loop. The error comes up on the second to last line.

  function setMarkers(map, markers) {

    for (var i = 0; i < markers.length; i++) {
        var sites = markers[i];
        var siteLatLng = new google.maps.LatLng(sites[1], sites[2]);
        var marker = new google.maps.Marker({
            position: siteLatLng,
            map: map,
            title: sites[0],
            zIndex: sites[3],
            html: sites[4],
            icon: featureImage
        });

        var contentString = "Some content";

        google.maps.event.addListener(marker, "click", function () {
            infowindow.setContent(this.html);
            infowindow.open(map, this);
        });
    }

How do I fix this error?

Community
  • 1
  • 1
user1876913
  • 77
  • 6
  • Except that the solutions in that question are needlessly complicated for the use-case above. This user **doesn't** need to create handlers that are custom to each loop iteration. – T.J. Crowder Dec 06 '12 at 16:54

1 Answers1

2

The warning is actually saying exactly what it means. :-) You have a for loop, and within the for loop you're creating a function to hand to addEventListener. That's frequently (though not always) an error, which is why it gets flagged up.

In your case, it would actually be harmless — but it looks as though you can just use one function for all of the elements you're creating in the loop rather than making more than one. So:

function setMarkers(map, markers) {

    for (var i = 0; i < markers.length; i++) {
        var sites = markers[i];
        var siteLatLng = new google.maps.LatLng(sites[1], sites[2]);
        var marker = new google.maps.Marker({
            position: siteLatLng,
            map: map,
            title: sites[0],
            zIndex: sites[3],
            html: sites[4],
            icon: featureImage
        });

        var contentString = "Some content";

        google.maps.event.addListener(marker, "click", handler);
    }

    function handler() {
        infowindow.setContent(this.html);
        infowindow.open(map, this);
    }
}

Now you're just creating the one function per call to setMarkers and reusing it.

T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875
  • 1
    @user1876913: A pleasure! As you said you're quite new at JavaScript, I'll just point out that the above makes use of something called a *closure* (the `handler` function "closes over" the `map` argument used in the call to `setMarkers` that created it). People get confused by closures, but they're actually not complicated. In fact, I wrote an article called *[Closures are not complicated](http://blog.niftysnippets.org/2008/02/closures-are-not-complicated.html)* because I saw so much confusion. :-) – T.J. Crowder Dec 06 '12 at 17:05
  • That's great, my head starts to spin with that. I'll definitely read this. Thanks for the link! – user1876913 Dec 06 '12 at 19:28