search

gordonbanderson / Mappable

Mappable module for SilverStripe CMS
BSD 3-Clause "New" or "Revised" License
10 stars 2 forks source link

Make MapAPI chainable #6

Open wernerkrauss opened 9 years ago

wernerkrauss commented 9 years ago

Will break BC but allow us to do something like $map = $myList->getRenderableMap()->setZoom(4)->setClusterer(true);

gordonbanderson commented 9 years ago

If likes of this in MapAPI

public function setZoom( $zoom ) {
    $this->zoom = $zoom;
  }

was changed to

public function setZoom( $zoom ) {
    $this->zoom = $zoom;
    return $this;
  }

would it allow the chain-ability whilst maintaining backwards compatibility?

wernerkrauss commented 9 years ago

For a given time sure, but like e.g. form fields handle it with returning a new, cloned object it just reduces side effects. Maybe not use "set" method names for that? How do other modules handle this?

gordonbanderson commented 9 years ago

In PHP5, return $this appears to return a reference, as such my suggested method appears correct. See http://stackoverflow.com/questions/3724112/php-method-chaining and http://stackoverflow.com/questions/7549423/how-do-i-chain-methods-in-php

gordonbanderson commented 9 years ago

I've pushed this change to the ISSUE6 branch and it appears to work as expected