[PEPr] Comment on Web Services::Services_Openstreetmap
| From: | Daniel O'Connor | Date: | Sat, 22 Oct 2011 00:43:58 +0000 |
| Subject: | [PEPr] Comment on Web Services::Services_Openstreetmap | ||
| References: | 1 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-54539@lists.php.net to get a copy of this message | ||
Hey Ken,
Looks good overall.
Given that there's loads of API; Services_Openstreetmap is sort of blending
a few roles - managing http transport, configuration, and some object
building & configuration.
It might be worth splitting this slightly - Services_Openstreetmap on
factory work; Services_Openstreetmap_Api to model all of the OSM API, and
something else which orchestrates the HTTP transport and the actual API
calls available.
IE: decoupled so much that you could add in different versions of the API
by defining a new Services_Openstreetmap_Api version if you were so
inclined.
I say this because there's a few places where you are instantiating and
configuring code (createChangeset, createNode, getUser, etc); dealing with
transport (getUser, loadXML, getResponse, _getObject) and other areas where
you are doing API work - getHistory, getWay, etc.
Those are somewhat seperate roles.
Other specifics:
I'd avoid doing things like:
Services_Openstreetmap_Objects
public function setXml($xml)
$cxml = simplexml_load_string($xml);
... in favour of
public function setXML(SimpleXMLElement $sxe)
Services_Openstreetmap
I'd make HTTP_Request2 injectable (you've done it a little bit with
injectable adapters), avoiding instantiation.
--
http://pear.php.net/pepr/pepr-proposal-show.php?id=667