note about computing centroid for polygons

[missinglink]

hi @paulmach

I got bit by a gotcha today, which although is technically correct, could cause confusion for some users.

When using the PointSet.Centroid() and PointSet.GeoCentroid() methods on a polygon, the calculated result was incorrect, for instance [1 1, 1 -1, -1 -1, -1 1, 1, 1] was not returning [0 0].

My error was that I failed to remove the last coordinate from the polygon (a duplicate of the first), which is a common convention to indicate that the polygon is closed (a convention in openstreetmap).

the fix was fairly simple:

isClosed = points.First().Equals(points.Last())
if isClosed {
  points.RemoveAt(len(*points) - 1)
}

The current implementation of numPoints := float64(len(ps)) is technically correct, but could possibly also bite others like me who aren't careful about it's usage.

How would you feel about me changing these calculations in PointSet to deduplicate the points before computing numPoints? This would likely slow down the code and require some memory to do the deduplication.

Other options include:

• Just dedupe the first and last points
• Add a Polygon type which has polygon specific code
• Write a few words about it in the code comments
• Not worry about it and hope that other people aren't big dummies like me :)

[paulmach]

hi @missinglink sorry for the late reply.

My feeling is the correct answer is "Add a Polygon type which has polygon specific code". If you're converting a polygon to a pointset it's up to the program to handle the edge cases.

The conventions in this library make it hard to add another type. I created a new library https://github.com/paulmach/orb a while back that does thing much better (in both a geo and go way) and I think handles this case much more cleanly.