Adding controls to OpenLayers maps - is this good code

Question

I have multiple OpenLayers maps, each map gets a subset of controls that I use across all maps. In the past I have added controls in the traditional way, for each control.

var control = new OpenLayers.Control.xxx({ options: whatever });
MAP.addControl(control);

I am considering defining an object that would contain most/all of the controls that I might use. That object would be is a .js file would be loaded into each web page. I would then add the controls I need for that page.

the control object, that is in the control.js file

var MY_controls = {
    scale:  new OpenLayers.Control.Scale(),
    mousePosition :new OpenLayers.Control.MousePosition({numDigits:6}),
    panZoomBar: new OpenLayers.Control.PanZoomBar(),
    ctrl_zoomin: new OpenLayers.Control.ZoomBox({ disable:true }),
    ctrl_zoomout: new OpenLayers.Control.ZoomBox({out:true,disable:true})
};

then on each page I would do something like this

array = ['scale','mousePosition','panZoomBar'];
for ( var x=0; x < array.length; ++x ) {
    MAP.addControl(MY_controls[array[x]]);
  };

Is this good code?

Answer 1

Assuming that by "I have multiple OpenLayers maps" you mean "multiple maps on a single page", one problem with the second approach is that a single control instance could end up being shared by multiple maps, leading to unexpected bugs/quirks (e.g. panning one map would move all maps that share the same control). A way to solve this would be to rewrite your code to spawn a new control each time instead of reusing the same instance:

// note that values in this map are not controls themselves,
// but factory functions that create new control instances
var MY_controls = {
    scale: function () {
        return new OpenLayers.Control.Scale();
    },
    mousePosition: function () {
        return new OpenLayers.Control.MousePosition({
          numDigits: 6
        });
    }
    // etc.
};

var array = ['scale','mousePosition','panZoomBar'];
for (var x=0; x < array.length; ++x ) {
    var ctrlFunction = MY_controls[array[x]];
    var ctrl = ctrlFunction.call();
    MAP.addControl(ctrl);
};

---

PS. I'd recommend using a for-each loop for looping through arrays (either browser's native or through UnderscoreJS), in your case:

var array = ['scale', 'mousePosition', 'panZoomBar'];
for (var x = 0; x < array.length; ++x) {
    MAP.addControl(MY_controls[array[x]]);
};

could be rewritten into:

_.each(['scale', 'mousePosition', 'panZoomBar'], function(ctrlName) {
    MAP.addControl(MY_controls[ctrlName]);
});