Introduce Error Codes #63
Conversation
|
|
Pull Request Test Coverage Report for Build 7999849463Details
💛 - Coveralls |
h3.go
Outdated
| @@ -67,6 +67,59 @@ const ( | |||
| RadsToDegs = 180.0 / math.Pi | |||
| ) | |||
|
|
|||
| var H3ErrorFailed = errors.New("Unknown error") | |||
There was a problem hiding this comment.
In addition to the following lint warnings, can you group them in a var block like
var (
ErrFailed = ...
ErrDomain = ...
..
)and update the error messages to start lowercase
https://github.com/golang/go/wiki/CodeReviewComments#error-strings
h3.go
Outdated
| var H3ErrorMemBound = errors.New("Memory bounds error") | ||
| var H3ErrorInvalidOption = errors.New("Invalid mode or flag error") | ||
|
|
||
| func New(errc int) error { |
There was a problem hiding this comment.
| func New(errc int) error { | |
| func errFromCode(errc int) error { |
suggest unexport the error mapper as it is not relevant to consumers and rename to reflect that it maps from h3 error codes from C to Go
h3.go
Outdated
| errc := C.latLngToCell(latLng.toCPtr(), C.int(resolution), &i) | ||
| if errc != C.E_SUCCESS { | ||
| return Cell(-1), New(int(errc)) | ||
| } | ||
|
|
||
| return Cell(i) | ||
| return Cell(i), nil |
There was a problem hiding this comment.
You can inline the check into the conditional:
if rc := C.latLngToCell(latLng.toCPtr(), C.int(resolution), &i); rc != C.E_SUCCESS {
return Cell(-1), errFromCode(int(rc))
}
return Cell(i), nilI've also renamed errc to rc as it is a return code, not necessarily an error.
It's a fun idea to introduce sentinel values Cell(-1), but the error we return is sufficient signal that we can keep the amount of change low by continuing to return empty/undefined values on error.
h3.go
Outdated
|
|
||
| var _ = H3ErrorFailed | ||
| var _ = H3ErrorDomain | ||
| var _ = H3ErrorLatLngDomain | ||
| var _ = H3ErrorResDomain | ||
| var _ = H3ErrorInvalidIndex | ||
| var _ = H3ErrorInvalidDirEdge | ||
| var _ = H3ErrorInvalidUnDirEdge | ||
| var _ = H3ErrorInvalidVertex | ||
| var _ = H3ErrorPentagon | ||
| var _ = H3ErrorDupInput | ||
| var _ = H3ErrorNeighbors | ||
| var _ = H3ErrorResMismatch | ||
| var _ = H3ErrorMemAlloc | ||
| var _ = H3ErrorMemBound | ||
| var _ = H3ErrorInvalidOption |
h3_test.go
Outdated
| assertEqual(t, 2, len(gb), "edge has 2 boundary cells") | ||
| }) | ||
| } | ||
| func testDirectedEdgeError(t *testing.T) { |
There was a problem hiding this comment.
| func testDirectedEdgeError(t *testing.T) { | |
| func TestDirectedEdgeError(t *testing.T) { |
h3_test.go
Outdated
| _, err := Cell(0x891ea6d6533ffff).DirectedEdges() | ||
| assertTrue(t, errors.Is(err, H3ErrorInvalidDirEdge)) | ||
|
|
||
| _, err = validCell.DirectedEdge(validDiskDist3_1[2][0]) | ||
| assertTrue(t, errors.Is(err, H3ErrorNeighbors)) | ||
|
|
||
| _, err = DirectedEdge(0).Origin() | ||
| assertTrue(t, errors.Is(err, H3ErrorInvalidDirEdge)) | ||
| _, err = DirectedEdge(0x115283773fffffff).Origin() | ||
| assertTrue(t, errors.Is(err, H3ErrorFailed)) | ||
|
|
||
| _, err = DirectedEdge(0).Destination() | ||
| assertTrue(t, errors.Is(err, H3ErrorInvalidDirEdge)) | ||
| _, err = DirectedEdge(0x115283773fffffff).Destination() | ||
| assertTrue(t, errors.Is(err, H3ErrorFailed)) | ||
|
|
||
| _, err = DirectedEdge(0).Cells() | ||
| assertTrue(t, errors.Is(err, H3ErrorInvalidDirEdge)) | ||
| _, err = DirectedEdge(0x115283773fffffff).Cells() | ||
| assertTrue(t, errors.Is(err, H3ErrorFailed)) | ||
|
|
||
| _, err = DirectedEdge(0x115283773fffffff).Boundary() | ||
| assertTrue(t, errors.Is(err, H3ErrorInvalidDirEdge)) |
There was a problem hiding this comment.
can you pop the literal addresses into a named fixture var at the top of the file, similar to the other fixture vars? it helps us keep a handle on what these values mean.
Introducing error codes from H3 v4. and Go error into APIs return signature.
Release note: breaking changes to almost all APIs.