[PEPr] Comment on Web Services::Services_Openstreetmap

From: 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

« previous php.pear.dev (#54539) next »