Handler class constructor optimization

[dziastinux]

Recently I was analyzing how handlers work and found interesting block of code:

/openlayers/lib/OpenLayers/Handler.js:

102 	    initialize: function(control, callbacks, options) {
103 	        OpenLayers.Util.extend(this, options);
104 	        this.control = control;
105 	        this.callbacks = callbacks;
106 	        if (control.map) {
107 	            this.setMap(control.map);
108 	        }
109 	
110 	        OpenLayers.Util.extend(this, options);
111 	       
112 	        this.id = OpenLayers.Util.createUniqueID(this.CLASS_NAME + "_");
113 	    },

For my opinion code at line 110 is redundant. Correct me if I am wrong.

[fredj]

remove redundant OpenLayers.Util.extend(this, options) in initialize

[fredj]

tests still pass

[elemoine]

fredj, with your patch handlers whose setMap method relies on options being set in the instance will be broken. So removing the second extend statement may make more sense.

[fredj]

Replying to elemoine:

fredj, with your patch handlers whose setMap method relies on options being set in the instance will be broken. So removing the second extend statement may make more sense.

You're right: attachment:"2137.1.patch" removes the second Util.extend.

Tests still pass.

[fredj]

(In [10408]) remove redundant OpenLayers.Util.extend. r=elemoine (closes #2137)

[fredj]

Regression with Overview Map:

• http://openlayers.org/dev/examples/overviewmap.html
• in the first map, zoom in twince
• in the overview map, click to recenter

The map is not correctly recentered

[fredj]

With the overview map control, the map is passed in the options argument and in the control argument.

With r10408, the second Util.extend was removed: the handler map is taken from the control, not from the options.

[fredj]

call setMap with options.map or control.map.

[elemoine]

Your patch looks good fredj. I'd like to see a comment above the declaration of the map property of the Handler class saying that providing a map option will make the constructor call setMap, and that the handler user is responsible for making sure setMap isn't called again on the handler. What do you think?

[fredj]

add a comment in the class constructor

[fredj]

(In [10434]) fix OverviewMap regression. r=elemoine (closes #2137)