Directive to create a Google map

Question

I have a considerably large directive which is doing many things: First it renders a Google Map, then it adds a listener to check when the bounds of the map change, then it renders points on the map based where the bounds currently are, and finally it has a menu of options which allow the user to turn on and off various settings on the map.

The code snippet below has been heavily modified to show the most important aspects of the directive. I've also hardcoded the template rather than used a templateURL.

This directive works exactly the way I want it to, and my questions are more about best practices for Angular:

1. Should I be pulling out the Ajax calls and placing them inside a service?
2. Should I create a Map controller and put some of this configuration logic in there?
3. When do I know if I need a new controller for that matter?

I'm afraid there is too much going on inside this directive and it should be broken out.

(function(angular) {
 'use strict';

 angular.module('myApp.directives').directive('myAppMap', [function() {

 return {
  template: '<div>A whole bunch of toggles for the map go here</div> \
             <div id="map-canvas" style="width: 100%; height: 600px"></div>'        
  link: function(scope, element, attrs) {

    var map, markers = [], currentLayer = [];

    function initialize() {
      var mapOptions = {
        zoom: 12,
        center: new google.maps.LatLng(34.05, -118.245),
        mapTypeId: google.maps.MapTypeId.ROADMAP,
        mapTypeControlOptions: {
          mapTypeIds: [google.maps.MapTypeId.ROADMAP, 'myApp_style']
        }
      },
      map = new google.maps.Map(document.getElementById('map-canvas'),mapOptions)
      map.mapTypes.set('myApp_style', styledMap);
      map.setMapTypeId('myApp_style');
      google.maps.event.addListener(map, 'bounds_changed', (function() {
       _getClusters();
      })());
    }

    function _getClusters(filters) {
      $.ajax({
        url: "http://localhost:1625/ocpu/library/imyApp/R/gen_cluster/json",
        type: 'POST',
        dataType:'json',
        data: {
          NE_lng:map.getBounds().getNorthEast(),
          SW_lng:map.getBounds().getSouthWest()
          maxNumber: 100
        },
        success:function(data){
          _setMarkers(map, JSON.parse(data)); 
        },
        error: function (xhr, ajaxOptions, thrownError) {
          console.log("ajax error - arun", xhr.status, thrownError);
        }
      })
    }

    function _setMarkers(map, clusters) {
      for (var i = 0; i < clusters.length; i++) {
        var thisCluster = clusters[i];
        var myLatLng = new google.maps.LatLng(thisCluster.lat, thisCluster.lon);
        markers[i] = new MarkerWithLabel({
          position: myLatLng,
          map: map,
          icon: "marker_01.png",
          labelClass: "myAppMarker",
          labelAnchor: new google.maps.Point(10, 40),
        })
        google.maps.event.addListener(markers[i], 'click', function() {
          map.setCenter(this.getPosition())
          map.setZoom(map.getZoom()+1);
        })
      }
    }

    function _toggleControls(direction) {
      if (direction=="open") {
        $(".mapControls").height("130");
        $(".mapControls .controls").fadeIn();   
      }
      else {
        $(".mapControls .controls").fadeOut();
        $(".mapControls").height("30");     
      }
    }

    $(".mapControls .open").click(function(){
      if ($(".mapControls .controls").is(":visible")) {
        _toggleControls("close")
      }
      else {
        _toggleControls("open")
      }
    })

    initialize();        

   }
 }
}]);
})(window.angular);

Answer 1

From a once over

• You seem to be missing a comma after </div>'
• You seem to be missing a comma after SW_lng:map.getBounds().getSouthWest()
• This code is reviewable, still, next time make sure there are no syntax errors..
• Hard coding (34.05, -118.245) is wrong, get it from your Angular model/controller
• Why prefix all your functions with _, except for initialize ?
• Hard coding "http://localhost:1625/ocpu/library/imyApp/R/gen_cluster/json" is bad
• Production code should not have console.log
• You are not using the variable currentLayer
• You are not using the parameter filters in _getClusters
• You should not create a new listener function for each marker in _setMarkers, you could create 1 function before the loop with a reference to map. This might cause performance problems
• You should merge _toggleControls and $(".mapControls .open").click(, it does not look DRY

All in all, I find the Google Maps API unwieldy, and because of that I do not think this is too much. However, you do need to something with the configuration logic, right now it looks akward.