Add error return codes to compactCells #468
Conversation
227e7d6 to
879ac99
Compare
| int64_t childrenSz = H3_EXPORT(uncompactCellsSize)(&origin, 1, 2); | ||
| int64_t childrenSz; | ||
| t_assertSuccess( | ||
| H3_EXPORT(uncompactCellsSize)(&origin, 1, 2, &childrenSz)); |
There was a problem hiding this comment.
For my info: Is there a clear guideline on when to use t_assertSuccess and when to use t_assert(foo(bar) == E_SUCCESS)? I'm assuming that the former is for expected successes in setup code, while the latter is for expected successes in the logic actually under test.
There was a problem hiding this comment.
This is rouhgly my usage. There may be some cases where I use t_assertSuccess for logic under test, but usually not so as to preserve the message. I think it can be treated as just a convenient way to write the t_assert invocation, as the messages are fairly opaque anyways (unless the message is used to clarify what is going on in the test)
src/h3lib/lib/h3Index.c
Outdated
| int64_t i = 0; | ||
|
|
||
| for (int64_t j = 0; j < numCompacted; j++) { | ||
| if (!_hasChildAtRes(compactedSet[j], res)) return -2; | ||
| if (!_hasChildAtRes(compactedSet[j], res)) return E_FAILED; |
There was a problem hiding this comment.
Nit: Do we want a more specific error here? E.g. E_RES_INVALID?
There was a problem hiding this comment.
I used the E_RES_MISMATCH for these cases, since it covers the H3Index res being invalid. There is a bit of overlap between E_RES_MISMATCH (the index resolution is out of range) and E_RES_DOMAIN (the resolution argument itself is out of range), which is tricky.
src/h3lib/lib/h3Index.c
Outdated
| int64_t numOut = 0; | ||
| for (int64_t i = 0; i < numCompacted; i++) { | ||
| if (compactedSet[i] == H3_NULL) continue; | ||
| if (!_hasChildAtRes(compactedSet[i], res)) return -1; // Abort | ||
| if (!_hasChildAtRes(compactedSet[i], res)) return E_FAILED; // Abort |
| @@ -325,7 +325,7 @@ function example() { | |||
|
|
|||
| Uncompacts the set `compactedSet` of indexes to the resolution `res`. `h3Set` must be at least of size `uncompactCellsSize(compactedSet, numHexes, res)`. | |||
|
|
|||
| Returns 0 on success. | |||
| Returns 0 (`E_SUCCESS`) on success. | |||
There was a problem hiding this comment.
... or non-zero error code on failure? Ideally we'd list out known errors here, linking to the error docs.
There was a problem hiding this comment.
Still thinking about how to add the known failure codes in here, but I agree it would be a useful addition.
2a886a5 to
49a9a00
Compare
website/docs/core-library/usage.md
Outdated
| @@ -7,7 +7,7 @@ slug: /core-library/usage | |||
|
|
|||
| ## API Versioning | |||
|
|
|||
| The public API of the H3 Core Library is defined in the file `h3api.h`. The functions defined in h3api.h adhere to [Semantic Versioning](http://semver.org/). | |||
| The public API of the H3 Core Library is defined in the file [`h3api.h`](https://github.com/uber/h3/blob/master/src/h3lib/include/h3api.h.in). The functions defined in `h3api.h` adhere to [Semantic Versioning](http://semver.org/). | |||
There was a problem hiding this comment.
Since we're linking to the .in file that we preprocess (not using the C preprocessor) maybe we need to explain that a bit?
No description provided.