Add error returns to directed edge functions #509
Conversation
src/h3lib/lib/directedEdge.c
Outdated
| H3Index origin; | ||
| H3Error originResult = H3_EXPORT(getDirectedEdgeOrigin)(edge, &origin); | ||
| if (originResult) { | ||
| // TODO: Not coverable |
There was a problem hiding this comment.
Nit: We could make this coverable by dropping the E_DIR_EDGE_INVALID check in this function and delegating that to getDirectedEdgeOrigin. The cost of the allocations we do before that call seems minimal.
src/h3lib/lib/directedEdge.c
Outdated
| H3Index origin; | ||
| H3Error originResult = H3_EXPORT(getDirectedEdgeOrigin)(edge, &origin); | ||
| if (originResult) { | ||
| // TODO: Unreachable |
There was a problem hiding this comment.
Again, we could drop the corresponding H3_DIRECTEDEDGE_MODE check within this function, possibly moving this call to the beginning, for full coverage.
| CellBoundary cb; | ||
|
|
||
| H3_EXPORT(directedEdgeToBoundary)(edge, &cb); | ||
| H3Error err = H3_EXPORT(directedEdgeToBoundary)(edge, &cb); | ||
| if (err) { |
There was a problem hiding this comment.
Nit: I find this reads much more clearly as err than result as you've used elsewhere. We should probably be consistent here, but I'd suggest either standardizing on err or funcErr or IS_H3_ERROR(funcResult) to make it clearer to the reader that if we have a truthy return value, it indicates an error.
WIP while working on how to handle coverage for this. There are a few branches which I know to be unreachable because the checks are duplicated.