golang / geo

S2 geometry library in Go
Apache License 2.0
1.69k stars 182 forks source link

Don't Swallow Error on Loop Validity #39

Closed jstaehle closed 6 years ago

jstaehle commented 6 years ago

Ran into an issue with a degenerate loop yesterday and found myself having to edit source code to show me useful errors for what was causing it to be invalid. Seems like there's a perfectly reasonable place to return a useful error here rather than swallow it, wonder if this is something that can be looked at?

https://github.com/golang/geo/blob/a8523298cefedcf7b70bbbf4eeef24cbb3258376/s2/loop.go#L207

jstaehle commented 6 years ago

Or perhaps another solution is to export this method so it can be used outside of this package:

https://github.com/golang/geo/blob/a8523298cefedcf7b70bbbf4eeef24cbb3258376/s2/loop.go#L214

dsymonds commented 6 years ago

Yeah, @rsned and I had discussed simply exporting findValidationError as Valid (or Check or something) and dropping IsValid. The only real reason we didn't was that C++ doesn't export the bool, but it seems a useful spot to diverge.