[OpenLayers-Dev] [vector-behavior] proposal for changes

[Tim Schaub]

Hey-

Thanks for summarizing your ideas Eric.

Eric Lemoine wrote:
> Hello
> 
> I'd like to bring a few changes to the vector-behavior code. Before
> making these changes, I wanted to describe them and give everyone a
> chance to speak up. Here they are:
> 
>  * remove update() and commit() from the strategy API. Just like
> controls and handlers, strategies are activated/deactivated through
> activate()/deactivate().

Agreed.  This is already how we're using strategies.

> 
>  * remove property 'strategy' from Layer.Vector, strategies are set in
> the 'strategies' properties (Array)

Also agreed.

> 
>  * make Layer.Vector responsible for creating, destroying, activating
> and deactivating its strategies. (a) Today Layer.Vector.setMap()
> activates the strategies but never deactivates them; instead,
> Layer.Vector should override removeMap() and deactivates the
> strategies in there. (b) To make Layer.Vector responsible for creating
> and destroying the strategies, Strategy classes as opposed to Strategy
> instances must be passed to the Layer.Vector constructor, I would go
> with:

I agree with the layer activating and deactivating strategies.  We could 
also include an autoActivate property (default is true).

On strategy destroy, I'd like to talk a bit more.  I know passing 
classes and options is an explicit way to ensure nobody is left with a 
reference to a destroyed instance - but it is also a bit restrictive.

The paging strategy (for example) exposes some stuff that is not easily 
accessible through the layer.  Chris and I talked about this and thought 
that having application developers keep a reference to the strategy was 
a good solution.  So you could do things like:

strategy.pageNext();

The alternative is to put method(s) on the layer that either 1) expose 
the strategies in a handy way, or 2) expose strategy methods.

An implementation of 1 would let you do something like:

layer.getStrategiesByClass(/Paging/)[0].pageNext();

The shortcoming here of course is that you're not guaranteed to get the 
right strategy.

An implementation of 2 would let you do something like:

layer.pageNext();

This is originally how I wrote the paging strategy (it would add methods 
to the layer).  Since this was considered distasteful, I changed it in 
favor of forcing app developers to keep a reference to the strategy.

I think the current solution is a better one (keeping a reference to the 
strategy and calling methods directly on it) than either 1 or 2 above.

Interested to hear what you (and others) think.

I don't really mind if the layer destroys strategies (and protocols) 
even if it didn't create them.  I know Erik (and likely others) don't 
like this - with good reason.

A third solution (which went nowhere before and would be a radical 
departure from the current coding style) would be to write some adapters 
that would allow something like:

IPaging(layer).pageNext();

(This of course requires on paging strategy per layer - in which case 
solution 1 above would always work anyway.)

Anyway, some things worth considering before passing functions instead 
of objects.

Tim


> 
> new OpenLayers.Layer.Vector({
>     strategies: [{
>             class: OpenLayers.Strategy.BBOX,
>             options: {ratio: 4}
>         }, {
>             class: OpenLayers.Strategy.Save
>     }]
> });
> 
>  * make Layer.Vector responsible for creating and destroying its
> protocol. In the same way as above, a Protocol class as opposed to a
> Protocol instance is passed to the Layer.Vector constructor:
> 
> new OpenLayers.Layer.Vector({
>     strategies: [{
>             class: OpenLayers.Strategy.BBOX,
>             options: {ratio: 4}
>         }, {
>             class: OpenLayers.Strategy.Save
>     }],
>     protocol: OpenLayers.Protocol.HTTP,
>     protocolOptions: {url: /path/to/service}
> });
> 
>  * remove the setLayer() method from the Strategy base class.
> Vector.Layer will do something like that:
> 
> for (var i = 0; i < this.strategies.length; i++) {
>     var strategy = new this.strategies[i].class({
>         layer: this,
>         ...
>     });
> }
> 
>  * make Layer.Vector responsible for creating/destroying the format
> instance. Again, a format class as opposed to a format instance is
> passed to the Layer.Vector constructor.
> 
>  * remove the setFormat() method from the Protocol class. The format
> instance is passed at protocol creating using new
> OpenLayers.Protocol({format: format}).
> 
> Thoughts?
> 
> Thanks,
> 
> --
> Eric
> _______________________________________________
> Dev mailing list
> Dev at openlayers.org
> http://openlayers.org/mailman/listinfo/dev
> 
> !DSPAM:4033,487c5cdb99535409313003!
>