Reference for last point input to PolyUtil.simplify() can change

[barbeau]

I was looking at PolyUtil.simplify() for polygons, and I realized there is a corner case related to the input that the current implementation doesn't handle properly.

As discussed in #201 (comment), to support simplifying polygons we ask developers to provide a closed polygon as input. Internally, we check if the first and last points have equal lat/longs (i.e., confirm its a closed polygon), and then if so we add a small offset to the last point to allow Douglas-Peucker to run on the polygon. We make this modification to the List in place to avoid having to copy the input list (time complexity of Douglas-Peucker is already O(n^2), and we don't want to add more overhead). Then, before returning we swap the modified last point out for the original last point.

The problem is that the current implementation makes this swap prior to returning by value (new LatLng object), not by reference. So, if the developer was holding onto a reference for the last LatLng object in the list before calling PolyUtil.simplify() , it wouldn't match the reference of the last point in the input List after PolyUtil.simplify() returns.

In other words:

ArrayList<LatLng> poly = new ArrayList<>();

LatLng p1 = new LatLng(28.06025, -82.41030);
LatLng p2 = new LatLng(28.06129, -82.40945));
LatLng p3 = new LatLng(28.06206, -82.40917));
LatLng p4 = new LatLng(28.06273, -82.40925));
LatLng p5 = new LatLng(28.06025, -82.41030);

poly.add(p1);
poly.add(p2);
poly.add(p3);
poly.add(p4);
poly.add(p5);

assertTrue(poly.get(poly.size() - 1) == p5);  // Passes

PolyUtil.simplify(poly, 10);

assertTrue(poly.get(poly.size() - 1) == p5);  // Fails

The original assertInputUnchanged() test didn't catch this because the Java assertEquals() uses LatLng.equals() to evaluate if the list contents are the same. And, LatLng.equals() returns true as long as the lat/long match, but doesn't check the object reference.

I'll submit a fix for this shortly, including improving the assertInputUnchanged() test so it also checks if the object references in the input list before and after execution are the same.

[barbeau]

PR with fix open at #205