Some improvements

[martinlindhe]

To simplify usage, i added a importPolyObjects() method, which allows the user to convert an array of objects to a polyline as long as the objects has the latitude & longitude properties set.

This patch series also contains some code cleanups: tabs -> spaces inconsistent opening function bracket { -> used zend convention

Removal of the polyline() function which had 3 different uses embedded inside of it: parse array => importPolyArray() parse string => importPolyString() get existing polyline node => getNode()

I updated the phpunit tests, removed incorrect @covers directives in order to generate a correct code coverage report, and enabled code coverage report generation in phpunit.xml.dist

[emcconville]

Thinks for your work! This PR has way to much going on for me to merge. Please isolate each change in a single PR (e.g. whitespace cleaning here, and unittest changes there &tc.) I'd be happy to include them.

As this PR stands, I can not merge until a few things get fixed.

• The untangle of polyline has broken legacy functionality. Please ensure the migration to import/node methods allows for backwards support.
• CodeCoverage in unit testing needs to meet, or beat, existing coverage rates. Just add a couple extra test to ensure excepted exceptions are supported.
• Please create an issues describing how @depends & @covers directives are confusing phpunit.
• Add documentation & example to new methods.

For the importPolyObjects. I'm simply not a fan of property check on an anonymous object. It simply locks users into a behavior that's not clear (unless they've read the src/doc). Let's create an interface to communicate what a PolyObject is.

// src/PolyObjectInterface.php
interface PolyObjectInterface
{
    public function getLatitude();
    public function getLongitude();
}
// Polyline::importPolyObjects
foreach ($objs as $obj) {
    if ($obj instance of PolyObjectInterface) {
         $arr[] = array($obj-> getLatitude(), $obj-> getLongitude());
    } else {
        trigger_error(/.../);
    }
}

Better yet, a tuple would be better suited.

// src/PolyObjectInterface.php
interface PolyObjectInterface
{
    public function getPoint();
}
// Polyline::importPolyObjects
foreach ($objs as $obj) {
    if ($obj instance of PolyObjectInterface) {
         $arr[] = $obj->getPoint();
    } else {
        trigger_error(/.../);
    }
}

Honestly, the whole singleton concept was a short lived fad, so I'm more than happy to deprecate all of that for a clean design pattern. Check out polyline-encoder, which is the library that is planed to surpass this one.

[martinlindhe]

Thanks for your reply and all your feedback! I agree, there is way too much going on in there. I should really have used a git branch but im fairly new to that, but this would be a good exercise. I cant defend why i dropped the polyline() function, i was not considering legacy use. So let me try clean this up and re-send in smaller chunks.

As for the future of the code, I was looking at something for handling polyline encoding and this solved it for me in a okay way. I also looked at your polyline-encode but it was basically just the string encode/decode functions. I prefered this lib since it was more "ready to use".

I would rework the class and drop the singleton stuff alltogether, but if you were working on it in another project, maybe i should just leave it at that.

As for documentation & example to new methods: all new methods should have comments and corresponding unit tests showing their use.

About the Interface, i have to sleep on that one. I'm not relly using interfaces in my php code. My use case for the importPolyObjects() method is just that i have many different php classes that have latitude & longitude properties, and these are mostly encoded in json, so getters & setter methods on these "thin" classes seems overkill to me. The naming convention of latitude & longitude i think is acceptable, since this lib is aimed for specifically the google maps api user, who would probably have such classes.

[emcconville]

Yes, please create a unique branch & PR. Way to easy not to do. Rebase & redo the whitespace stuff to get started. I'll draft a milestone to outline what we'll be deprecating. Ideally the core object should only handle encode/decode, and everything else should be delegate out to other design patterns.

I know your thinking latitude(Y) & longitude(X) is enough, but the algorithm can support altitude(Z) & path distance (M). This lib will always be intended for Google API, but shouldn't be limited to it.

[martinlindhe]

I have now done as you asked. However i think i made a mistake with the git workflow. Now i sent 3 branches for review/accept, but they are depending on eachother. The 2nd takes off where the 1st left, and the 3rd take off after the second. I get the feeling i should made them independent of eachother somehow, however they also are a related series of changes. Perhaps i should do some rebasing? (I'm a bit new to the git stuff.. :-)