Add DumpCoordinates methods
#398
Conversation
These have the same semantics, and will be more efficient.
| @@ -18,3 +25,76 @@ type Coordinates struct { | |||
| // or not Z and M are populated. | |||
| Type CoordinatesType | |||
| } | |||
|
|
|||
| // String gives a string representation of the coordinates. | |||
| func (c Coordinates) String() string { | |||
There was a problem hiding this comment.
Need to add a test for this.
geom/type_coordinates.go
Outdated
| } | ||
|
|
||
| // NewXYCoordinates constructs a new set of coordinates of type XY. | ||
| func NewXYCoordinates(x, y float64) Coordinates { |
There was a problem hiding this comment.
These probably aren't necessary anymore... I used then for tests before I switched from returning []Coordinates to returning Sequence. Can be removed.
| case TypePoint: | ||
| return g.AsPoint().DumpCoordinates() | ||
| case TypeLineString: | ||
| return g.AsLineString().Coordinates() |
There was a problem hiding this comment.
There is no DumpCoordinates method for LineString, because the existing Coordinates method does the exact same thing. Even though it's a bit inconsistent, I think that's less confusing than having two methods that do exactly the same thing.
There was a problem hiding this comment.
if each geometry implemented DumpCoordinates(), could we do something like g.AsDumpable().DumpCoordinates() (for want of a better function name) and not required the switch-case block?
There was a problem hiding this comment.
That (or something similar) might be possible. The challenge would be implementing AsDumpable() on the Geometry type. The obvious way to implement it would be to use a switch, but that would defeat the point since we're trying to eliminate a switch statement. It may be possible to use some sort of reflect/unsafe magic though.
The "switch" pattern actually shows up extremely frequently in this file (and is a bit of a code smell). Possibly it could be implemented generically so that we only need a single switch statement for all interface types.
Thanks for bringing this up! 😄 I've raised a ticket here: #399
These aren't really necessary at this point. We may wish to add something like these in the future though.
Description
Adds
DumpCoordinatesmethods on geometry types. These methods give back aSequencethat contains all points in the geometry.Check List
Have you:
Added unit tests? Yes.
Add cmprefimpl tests? (if appropriate?) N/A
Related Issue
Benchmark Results
N/A