Handle empty (zero length) input to compact
#278
Conversation
`compact`, `maxUncompactSize`, and `uncompact` should handle the case where empty input is provided. In this case the arrays should not be dereferenced, since the first index is already out of range. This also adds tests for some uncovered branches.
src/apps/testapps/testPolygon.c
Outdated
| t_assert(countLinkedPolygons(&polygon) == 1, "Polygon count correct"); | ||
| t_assert(countLinkedLoops(&polygon) == 1, | ||
| "Loop count on first polygon correct"); | ||
| t_assert(polygon.first == outer, "Got expected outer loop"); |
There was a problem hiding this comment.
Is it worth asserting output here? My assumption would be that if we are returning error codes, we relinquish all guarantees about output when you get one - so no behavior after an error code is returned should be assertable, because it's all undefined.
There was a problem hiding this comment.
It may not be, since the polygon doesn't have all the features anymore.
edit: removed the asserts about the properties of the polygon.
| @@ -266,6 +266,9 @@ H3Index H3_EXPORT(h3ToCenterChild)(H3Index h, int childRes) { | |||
| */ | |||
| int H3_EXPORT(compact)(const H3Index* h3Set, H3Index* compactedSet, | |||
| const int numHexes) { | |||
| if (numHexes == 0) { | |||
There was a problem hiding this comment.
Do we need similar handling on uncompact?
There was a problem hiding this comment.
uncompact and maxUncompactSize have for loops like for (int i = 0; i < numHexes; i++), so they are already exiting when the number of hexagons is 0. I added tests in this PR to confirm this.
| @@ -266,6 +266,9 @@ H3Index H3_EXPORT(h3ToCenterChild)(H3Index h, int childRes) { | |||
| */ | |||
There was a problem hiding this comment.
Can we be explicit about what codes are returned in success and failure cases?
There was a problem hiding this comment.
So far we haven't done that in the public API, I think with the idea that the error codes might change in patch changes. The error codes have been used in some of the bindings to generate exception messages, though, so they are useful.
Handle empty (zero length) input to `compact`
compact,maxUncompactSize, anduncompactshould handle thecase where empty input is provided. In this case the arrays
should not be dereferenced, since the first index is already out
of range.
This also adds tests for some uncovered branches.