0
\$\begingroup\$

I'm a mediocre programmer at best and am pretty terrible at JavaScript. But I have been tasked with creating a Google Maps application. This application is driven by a dynamic report with a Java layer on top of it. This Java layer spits out generated Google API JavaScript.

The problem I am running into is that I am attempting to plot a huge number of markers. I am already investigating how to set the markers map to null using AJAX. But I am wondering if someone could look at this code and tell me if anything is redundant. When my application is not restricted it could easily spit out 1000 of each of these blocks, which results in a ton of JavaScript.

Below is a sample run of my code:

<!DOCTYPE html>        
<html>    
  <head>  
    <meta name="viewport" content="initial-scale=1.0, user-scalable=no">
    <meta charset="utf-8">
    <title>txtMark</title>
    <style>
        #map-canvas {
            height: 500px;
            width: 500px;
    </style>    
    <script src="https://maps.googleapis.com/maps/api/js?v=3.exp&sensor=false"></script>
    <script>     
       var txtMark = {};
       var circles = {};
       var balloons = {};
       var highPin = 'http://chart.apis.google.com/chart?chst=d_map_pin_letter&chld=%E2%80%A2|008000';
       var lowPin = 'http://chart.apis.google.com/chart?chst=d_map_pin_letter&chld=%E2%80%A2|FFFF00';
       var medPin = 'http://chart.apis.google.com/chart?chst=d_map_pin_letter&chld=%E2%80%A2|FE7569';
       var map;

       function initialize() {
           var mapOptions = {
              zoom: 8,
            center: new google.maps.LatLng(38.873555, -77.295380),
            mapTypeId: google.maps.MapTypeId.ROADMAP
        };

        map = new google.maps.Map(document.getElementById('map-canvas'), mapOptions);

        circles[0] = {
            center: new google.maps.LatLng(38.873555, -77.295380),
            id: 0,
            addr: '00601',
            r: 16093.4,
            txt: '00601: Claims With Labor:322',
            color: '#FE7569'
        };

        balloons[0] = {
            center: new google.maps.LatLng(38.873555, -77.295380),
            id: 0,
            pin: lowPin,
            addr: '00601',
            txt: '00601: Claims With Part And Labor:139'
        };

        txtMark[0] = {
            center: new google.maps.LatLng(38.873555, -77.295380),
            id: 0,
            style: 'FE7569',
            addr: '139',
            txt: '00601: Claims With Part Only:139'
        };

        var tInfo = new google.maps.InfoWindow();
        for (i in txtMark) {
            var tMark = new MarkerWithLabel({
                position: txtMark[i].center,
                map: map,
                labelClass: "labels",
                labelStyle: {
                    color: txtMark[i].style
                },
                labelContent: txtMark[i].addr,
                icon: {}
            });

            google.maps.event.addListener(tMark, 'click', (function (tMark, i) {
                return function () {
                    if (tInfo) {
                        infoWindow.close();
                        tInfo.close();
                        bInfo.close();
                    }
                    tInfo.setContent(txtMark[i].txt);
                    tInfo.setPosition(txtMark[i].center);
                    tInfo.open(map);
                }
            })(tMark, i));
        }

        var bMarker;
        var bInfo = new google.maps.InfoWindow();
        for (i in balloons) {
            var balloonOptions = {
                map: map,
                id: balloons[i].id,
                position: balloons[i].center,
                icon: balloons[i].pin,
                infoWindowIndex: i
            };

            bMarker = new google.maps.Marker(balloonOptions);
            google.maps.event.addListener(bMarker, 'click', (function (bMarker, i) {
                return function () {
                    if (bInfo) {
                        infoWindow.close();
                       tInfo.close();
                        bInfo.close();
                    }
                    bInfo.setContent(balloons[i].txt);
                    bInfo.setPosition(balloons[i].center);
                    bInfo.open(map);
                }
            })(bMarker, i));
        }

        var cityCircle;
        var infoWindow = new google.maps.InfoWindow();
        for (i in circles) {
            var magnitudeOptions = {
                strokeColor: circles[i].color,
                fillColor: circles[i].color,
                map: map,
                center: circles[i].center,
                radius: circles[i].r,
                id: circles[i].id,
                addr: circles[i].addr,
                infoWindowIndex: i
            };

            cityCircle = new google.maps.Circle(magnitudeOptions);
            google.maps.event.addListener(cityCircle, 'click', (function (cityCircle, i) {
                return function () {    
                    if (infoWindow) {
                        infoWindow.close();
                        tInfo.close();
                        bInfo.close();
                    }
                    infoWindow.setContent(circles[i].txt);
                    infoWindow.setPosition(cityCircle.getCenter());
                    infoWindow.open(map);
                }
            })(cityCircle, i));
        }
    }

    google.maps.event.addDomListener(window, 'load', initialize)

    </script>
  </head>
  <body>
    <div id="map-canvas"></div>
  </body>
</html>
\$\endgroup\$
  • \$\begingroup\$ To clarify… you expect that sometimes there may be circles[0], circles[1], … circles[999] as well as many balloons[] and txtMark[]? Do the balloons[] and txtMark[] always coincide in location? \$\endgroup\$ – 200_success Nov 14 '13 at 21:09
  • \$\begingroup\$ There will always be the same number of circles, balloons, and txtMark. They are related as in they are plotted on the same lat/long. But dont necessarily have anything in common besides that. But yes there may be cirles[999], balloons[999], txtMark[999], and even a polygon[999] once I get the data. I know its a huge amount of markers and I am working to make some enhancements with ajax calls to reduce plotted points that arent in view. \$\endgroup\$ – user2524908 Nov 14 '13 at 21:11
  • \$\begingroup\$ But in your example, circles[0] is not in the same location as balloons[0] and txtMark[0]? \$\endgroup\$ – 200_success Nov 14 '13 at 21:14
  • \$\begingroup\$ Oh I am sorry. I changed it to test something else out. Let me fix that. \$\endgroup\$ – user2524908 Nov 14 '13 at 21:19
  • \$\begingroup\$ Any suggestions? \$\endgroup\$ – user2524908 Nov 15 '13 at 15:23
1
\$\begingroup\$

I can't really speak to performance issues, since there are too many unknowns. E.g. oes the user need 1000+ locations all at once? What is acceptable performance?
I will say that simply loading everything (and the kitchen sink) and plotting it all seems pretty inelegant. But coming up with a smarter solution would require knowledge of your problem domain.

But I'll speak to the code a bit. First some basic JS remarks:

  1. Use an IIFE to isolate your code from the global scope. Right now, all your variables are global.
  2. Use arrays. You're using objects, and iterating with for...in, which is - I'm sorry to say - plain wrong for what you're trying to achieve

As for structure: Judging by your exchange with 200_success in the comments, you will have N number of locations (lat/lng positions), and each of those will have a circle, a label and a balloon.

In that case, use objects to encapsulate all that. Then take those objects, and stick those in a (proper) array. Right now, you're duplicating data across those three things, and you're "manually" tracking 3 different "arrays" that aren't really arrays.

Here's the object structure I'd propose (I'll clean it up futher in a bit)

var locations = [ // an array!
  {
    id: 0, // I'm assuming the ID is tied to the lat/lng; not the individual circles/labels/balloons
    position: new google.maps.LatLng(38.873555, -77.295380),
    circle: {
      addr: '00601',
      r: 16093.4,
      txt: '00601: Claims With Labor:322',
      color: '#FE7569'
    },
    balloon: {
      pin: lowPin,
      addr: '00601',
      txt: '00601: Claims With Part And Labor:139'
    },
    txtMark: {
      style: 'FE7569',
      addr: '139',
      txt: '00601: Claims With Part Only:139'
    }
  },
  // more location objects here ...
];

Now locations[0] will be an object containing everything to do with that particular point on the globe.

But while we're at it, I highly suggest using different names for the "items": circle, balloon and txtMark do not describe the data, but their presentation. And that's a separate concern right now. Name data for what they represent, not how they are represented.

I'll stick with your names for this, though, as I don't know what these things should rightly be called (though partOnly, laborOnly and partAndLabor seem like obvious candidates).

Looking at the above, there's more repetition we can probably get rid of in the data objects.

{
  id: 0,
  position: new google.maps.LatLng(38.873555, -77.295380),
  address: '00601',
  style: 'A',
  circle: {
    radius: 16093.4,
    text: '00601: Claims With Labor:322'
  },
  balloon: {
    text: '00601: Claims With Part And Labor:139'
  },
  label: {
    text: '00601: Claims With Part Only:139'
  }
}

I've cleaned up some names, but I'm also making some assumptions here:

  1. addr is the same for all 3 even though your code has 139 as the txtMark's address. Again, your conversation with 200_success makes me think this might be a typo.
  2. The "style" is the same for all things at one location. I.e. a circle color #FE7569 implies a lowPin implies a FE7569-style label. Here, I've just called that style "A" (come up with a better name) and made it common to all 3.

For the styles, I'd make a single object, like so:

var styles = {
  A: {
    color: "#FE7569",
    pin: "http://chart.apis.google.com/chart?chst=d_map_pin_letter&chld=%E2%80%A2|FFFF00"
  },
  B: {
    color: ...
    pin: ...
  },
  // etc.
}

So that's the data set, now for the logic. Instead of looping through 3 different objects, you now only have to loop though 1 array, so:

var i, l, location, style, label, circle, balloon,
    infoWindow = new google.maps.InfoWindow(); // you only need 1 window!

// I'm leaving the map initilization out of it; just assume there's a `map` variable

// generalized click handler
function addClickHandler(item, content, position) {
  google.maps.event.addListener(item, 'click', function () {
    infoWindow.close();
    infoWindow.setContent(content);
    infoWindow.setPosition(position);
    infoWindow.open(map);
  });
}

// functions to add the items to the map

function addCircle(location, style) {
  var circle = new google.maps.Circle({
    map: map,
    center: location.position,
    radius: location.circle.radius,
    strokeColor: style.color,
    fillColor: style.color,
    addr: location.address // do you actually need this for anything?
  });
  addClickHandler(circle, location.circle.text, location.position);
}

function addLabel(location, style) {
  var label = new MarkerWithLabel({
    map: map,
    position: location.position,
    labelClass: "labels",
    labelStyle: { color: style.color },
    labelContent: location.address,
    icon: null // I'm assuming you can just do this to avoid a default icon
  });
  addClickHandler(label, location.label.text, location.position);
}

function addBalloon(location, style) {
  var balloon = new google.maps.Marker({
    map: map,
    position: location.position,
    icon: style.pin
  });
  addClickHandler(balloon, location.balloon.text, location.position);
}

// loop through the array
for( i = 0, l = locations.length ; i < l ; i++ ) {
  location = locations[i];
  style    = styles[location.style];
  addCircle(location, style);
  addLabel(location, style);
  addBalloon(location, style);
}

It's not perfect, but it's better. I haven't tested it it though, but the point is really about structure, and the structure should be ok

\$\endgroup\$
  • \$\begingroup\$ Hey I really appreciate you taking the time to outline the options and clearly explain them. As you can tell I am not much of a javascript programer. I will look into each of the options you have outlined. \$\endgroup\$ – user2524908 Nov 18 '13 at 17:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.