Comments for "Services_Openstreetmap"

» Submit Your Comment
Comments are only accepted during the "Proposal" phase. This proposal is currently in the "Finished" phase.
» Comments

  • Daniel O'Connor  [2011-10-22 00:43 UTC]

    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.

  • Till Klampaeckel  [2011-10-23 14:08 UTC]

    Hey Ken,

    thanks for proposing this, I think I pretty much second Daniel's suggestions.

    In addition, what do you think about providing more objects to the developer (vs. arrays of arrays). Just a small suggestion, not sure if you thought about it yet.

    So e.g. your results array would have objects, which I can query with getAddress(), etc.. You could of course ways to work with the result with something like ArrayAccess etc.. ;-)

    Just a suggestion.

    Also, great use of tests! :-)

    Till