Re-factor datasource bind() impl #962

Closed
artemp opened this Issue Nov 22, 2011 · 6 comments

Projects

None yet

2 participants

@artemp
Member
artemp commented Nov 22, 2011

Current implementation requires every member function to do something like :

 if(!is_bound)
 {
      this->bound(); 
      is_bound=true;
 }
.....

This makes implementing/maintaing datasources a cumbersome process - you have to remember to check state, call init function .

One solution would be to move bound / unbound logic to mapnik::Layer and to ensure one-step initialisation in datasource objects.

@artemp artemp was assigned Nov 22, 2011
@springmeyer
Member

Okay, starting to move on this cleanup at https://github.com/mapnik/mapnik/tree/move_bind_logic_to_layer. This will mean removing the bind argument.

It will mean that lazy/late initialization of datasources will be default - they will only be created when needed - at render time.

The previous functionality of bind=false (whereby datasources could be validated by not actually being bound) could potentially be maintained by adding a static ds.is_valid() function that checks for valid params being present before initialization.

@artemp
Member
artemp commented Mar 12, 2012

"bind-less" implementation : https://github.com/mapnik/mapnik/tree/move_bind_logic_to_layer

We can add 'bind' parameter to the layer object if needed.

@springmeyer
Member

This is waiting on me to merge into master after reviewing, adding backwards compatibility, and updating any tests that need it. thanks for your patience!

@artemp
Member
artemp commented Jul 3, 2012
@springmeyer springmeyer was assigned Jul 16, 2012
@springmeyer
Member

okay, just not getting to this, so pushing off milestone - will take a fresh look after 2.1 release. Still on my plate to do this.

@springmeyer springmeyer pushed a commit to mapnik/node-mapnik that referenced this issue Oct 8, 2012
Dane Springmeyer bind arg is going away in mapnik soon (mapnik/mapnik#962) so remove i…
…t - also closes #124
c86f390
@springmeyer springmeyer pushed a commit that referenced this issue Dec 17, 2012
Dane Springmeyer remove bind option for datasources - refs #962 c5410fa
@springmeyer
Member

closing, handled by #1654, #1655, and #1656.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment