[OpenLayers-Dev] constructor signatures

[christopher.schmidt at nokia.com]

On Jun 17, 2010, at 6:33 PM, ext Tim Schaub wrote:
>     var popup = new OpenLayers.Popup.FramedCloud({
>         location: xy,
>         text: text,
>         closeBox: true
>     });
> 
> I think the second form is more readable and less prone to errors.
> 
> The one argument constructor is nothing new.  We're doing it already in 
> some places in the library.  People coming from popular JavaScript 
> frameworks are comfortable with this.
> 
> In adding the WMTS layer, I created a constructor that takes a single 
> argument.  If others agree that this is the way forward, I think there 
> is sense in getting used to it before 3.0.
> 
> I am bringing this up here because Chris objected to my commit of the 
> WMTS layer and reopened the ticket.
> 
> If anybody else has an opinion about the many versus one argument 
> constructor, please weigh in.

In general, for 2.x, I am supportive of having required 
arguments as seperate parameters, and optional arguments in a single
object. Some of our existing classes -- popups being a big one :) --
are obviously not following this convention.

I do not think it is unreasonable to make some things 'required' even
if they are actually not technically required. In the WMS spec, the 
Title attribute is required, even though there is no technical reason
why this matters; it simply makes working with layers easier when
they have names, rather than just positions in an array.

When creating a new subclass, One of the first things that I do is
compare the Constructor signatures of the parent class to the
subclass. If the parent has a constructor which is not a 'subset' of
the child object, it usually means I've done something wrong. In the 
case of Layer, the parent class has:

  Layer(name, options)

My convention for subclasses, then, would be that I could put things
*between* 'name' and 'options', but that I could not get rid of those
things; Layer.WMS has:

  Layer(name, params, options);

In all cases I'm aware of (Though I have not examined filters and rules
extensively, so this may not be true in some portions of the code), 
all of our subclassed items may add things after the 'required' 
parameters, and before the 'options' parameter (which provides a
space for non-required parameters). 

I am comfortable with 'name' being treated as a 'required' parameter
of layers. I am in favor of maintaining the style of 'extending' 
parameters following this pattern in 2.x; specifically, if a 
parent class declares something 'required', the subclass should
generally treat it as 'required' (and therefore a seperate parameter)
unless doing so is significantly painful.

Backing out on something that is one of our few points of consistency --
Layers require a string as their first argument -- would make me 
sad, but I'm not going to argue that we need to revert a change
because of it. I just think it's a worthwhile convention to hold onto
until we really go ahead and change many of our conventions for the
Glorious Future.

Going forward into 3.x, I am okay with abandoning this convention
in favor of making everything optional. I don't like it, personally, 
but if this is common Javascript convention, I'm willing to go with 
it. 

Apologies if I nitpicked on this one specific feature; I can understand
that the WMTS layer was likely a lot of work, and am sure that it will
help many people. I just wanted to comment on the convention change 
because I thought it might actively trip people up.

Best Regards,
-- 
Christopher Schmidt
Nokia