Add error returns to gridPath/gridDistance functions

uber · isaacbrodsky · Sep 3, 2021 · Aug 24, 2021 · Aug 24, 2021 · Aug 24, 2021
commit 13c94cae6e9b2ae6e77d343cc8407d7112e48f0f
diff --git a/src/apps/benchmarks/benchmarkGridPathCells.c b/src/apps/benchmarks/benchmarkGridPathCells.c
@@ -23,8 +23,9 @@ H3Index endFar = 0x8929a5653c3ffff;
 
 BEGIN_BENCHMARKS();
 
-H3Index *out =
-    calloc(H3_EXPORT(gridPathCellsSize)(startIndex, endFar), sizeof(H3Index));
+int64_t size;
+H3_EXPORT(gridPathCellsSize)(startIndex, endFar, &size);
+H3Index *out = calloc(size, sizeof(H3Index));
 
 BENCHMARK(gridPathCellsNear, 10000, {
     H3_EXPORT(gridPathCells)(startIndex, endNear, out);

diff --git a/src/apps/testapps/testGridDistance.c b/src/apps/testapps/testGridDistance.c
@@ -63,15 +63,21 @@ SUITE(gridDistance) {
         H3Index p6;
         setH3Index(&p6, 1, 14, 6);
 
-        t_assert(H3_EXPORT(gridDistance)(bc, p) == 3, "distance onto pentagon");
-        t_assert(H3_EXPORT(gridDistance)(bc, p2) == 2, "distance onto p2");
-        t_assert(H3_EXPORT(gridDistance)(bc, p3) == 3, "distance onto p3");
+        int64_t distance;
+        t_assertSuccess(H3_EXPORT(gridDistance)(bc, p, &distance));
+        t_assert(distance == 3, "distance onto pentagon");
+        t_assertSuccess(H3_EXPORT(gridDistance)(bc, p2, &distance));
+        t_assert(distance == 2, "distance onto p2");
+        t_assertSuccess(H3_EXPORT(gridDistance)(bc, p3, &distance));
+        t_assert(distance == 3, "distance onto p3");
         // TODO works correctly but is rejected due to possible pentagon
         // distortion.
-        //    t_assert(H3_EXPORT(gridDistance)(bc, p4) == 3, "distance onto
-        //    p4"); t_assert(H3_EXPORT(gridDistance)(bc, p5) == 4, "distance
-        //    onto p5");
-        t_assert(H3_EXPORT(gridDistance)(bc, p6) == 2, "distance onto p6");
+        // t_assertSuccess(H3_EXPORT(gridDistance)(bc, p4, &distance));
+        // t_assert(distance == 3, "distance onto p4");
+        // t_assertSuccess(H3_EXPORT(gridDistance)(bc, p5, &distance));
+        // t_assert(distance == 4, "distance onto p5");
+        t_assertSuccess(H3_EXPORT(gridDistance)(bc, p6, &distance));
+        t_assert(distance == 2, "distance onto p6");
     }
 
     TEST(testIndexDistance2) {
@@ -80,20 +86,24 @@ SUITE(gridDistance) {
         H3Index destination = 0x821ce7fffffffffL;
 
         // TODO doesn't work because of pentagon distortion. Both should be 5.
-        t_assert(H3_EXPORT(gridDistance)(destination, origin) == -1,
+        int64_t distance;
+        t_assert(H3_EXPORT(gridDistance)(destination, origin, &distance) !=
+                     E_SUCCESS,
                  "distance in res 2 across pentagon");
-        t_assert(H3_EXPORT(gridDistance)(origin, destination) == -1,
+        t_assert(H3_EXPORT(gridDistance)(origin, destination, &distance) !=
+                     E_SUCCESS,
                  "distance in res 2 across pentagon (reversed)");
     }
 
     TEST(gridDistanceBaseCells) {
-        t_assert(H3_EXPORT(gridDistance)(bc1, pent1) == 1,
-                 "distance to neighbor is 1 (15, 4)");
-        t_assert(H3_EXPORT(gridDistance)(bc1, bc2) == 1,
-                 "distance to neighbor is 1 (15, 8)");
-        t_assert(H3_EXPORT(gridDistance)(bc1, bc3) == 1,
-                 "distance to neighbor is 1 (15, 31)");
-        t_assert(H3_EXPORT(gridDistance)(pent1, bc3) == -1,
+        int64_t distance;
+        t_assertSuccess(H3_EXPORT(gridDistance)(bc1, pent1, &distance));
+        t_assert(distance == 1, "distance to neighbor is 1 (15, 4)");
+        t_assertSuccess(H3_EXPORT(gridDistance)(bc1, bc2, &distance));
+        t_assert(distance == 1, "distance to neighbor is 1 (15, 8)");
+        t_assertSuccess(H3_EXPORT(gridDistance)(bc1, bc3, &distance));
+        t_assert(distance == 1, "distance to neighbor is 1 (15, 31)");
+        t_assert(H3_EXPORT(gridDistance)(pent1, bc3, &distance) != E_SUCCESS,
                  "distance to neighbor is invalid");
     }
 
@@ -119,8 +129,9 @@ SUITE(gridDistance) {
     }
 
     TEST(gridDistanceResolutionMismatch) {
-        t_assert(H3_EXPORT(gridDistance)(0x832830fffffffffL,
-                                         0x822837fffffffffL) == -1,
+        int64_t distance;
+        t_assert(H3_EXPORT(gridDistance)(0x832830fffffffffL, 0x822837fffffffffL,
+                                         &distance) == E_RES_MISMATCH,
                  "cannot compare at different resolutions");
     }
 
@@ -130,14 +141,15 @@ SUITE(gridDistance) {
         H3Index edge = H3_EXPORT(cellsToDirectedEdge)(origin, dest);
 
         t_assert(0 != edge, "test edge is valid");
-        t_assert(H3_EXPORT(gridDistance)(edge, origin) == 0,
-                 "edge has zero distance to origin");
-        t_assert(H3_EXPORT(gridDistance)(origin, edge) == 0,
-                 "origin has zero distance to edge");
-
-        t_assert(H3_EXPORT(gridDistance)(edge, dest) == 1,
-                 "edge has distance to destination");
-        t_assert(H3_EXPORT(gridDistance)(dest, edge) == 1,
-                 "destination has distance to edge");
+        int64_t distance;
+        t_assertSuccess(H3_EXPORT(gridDistance)(edge, origin, &distance));
+        t_assert(distance == 0, "edge has zero distance to origin");
+        t_assertSuccess(H3_EXPORT(gridDistance)(origin, edge, &distance));
+        t_assert(distance == 0, "origin has zero distance to edge");
+
+        t_assertSuccess(H3_EXPORT(gridDistance)(edge, dest, &distance));
+        t_assert(distance == 1, "edge has distance to destination");
+        t_assertSuccess(H3_EXPORT(gridDistance)(dest, edge, &distance));
+        t_assert(distance == 1, "destination has distance to edge");
     }
 }
diff --git a/src/apps/testapps/testGridDistanceExhaustive.c b/src/apps/testapps/testGridDistanceExhaustive.c
@@ -35,7 +35,9 @@
 static const int MAX_DISTANCES[] = {1, 2, 5, 12, 19, 26};
 
 static void gridDistance_identity_assertions(H3Index h3) {
-    t_assert(H3_EXPORT(gridDistance)(h3, h3) == 0, "distance to self is 0");
+    int64_t distance;
+    t_assertSuccess(H3_EXPORT(gridDistance)(h3, h3, &distance));
+    t_assert(distance == 0, "distance to self is 0");
 }
 
 static void gridDistance_gridDisk_assertions(H3Index h3) {
@@ -54,12 +56,16 @@ static void gridDistance_gridDisk_assertions(H3Index h3) {
             continue;
         }
 
-        int calculatedDistance = H3_EXPORT(gridDistance)(h3, neighbors[i]);
+        int64_t calculatedDistance;
+        H3Error calculatedError =
+            H3_EXPORT(gridDistance)(h3, neighbors[i], &calculatedDistance);
 
         // Don't consider indexes where gridDistance reports failure to
         // generate
-        t_assert(calculatedDistance == distances[i] || calculatedDistance == -1,
-                 "gridDiskDistances matches gridDistance");
+        if (calculatedError == E_SUCCESS) {
+            t_assert(calculatedDistance == distances[i],
+                     "gridDiskDistances matches gridDistance");
+        }
     }
 
     free(distances);

diff --git a/src/apps/testapps/testGridPathCells.c b/src/apps/testapps/testGridPathCells.c
@@ -34,7 +34,9 @@ SUITE(gridPathCells) {
         H3Index start = 0x85285aa7fffffff;
         H3Index end = 0x851d9b1bfffffff;
 
-        int lineSz = H3_EXPORT(gridPathCellsSize)(start, end);
-        t_assert(lineSz < 0, "Line not computable across multiple icosa faces");
+        int64_t size;
+        H3Error lineError = H3_EXPORT(gridPathCellsSize)(start, end, &size);
+        t_assert(lineError == E_FAILED,
+                 "Line not computable across multiple icosa faces");
     }
 }
diff --git a/src/apps/testapps/testGridPathCellsExhaustive.c b/src/apps/testapps/testGridPathCellsExhaustive.c
@@ -36,13 +36,13 @@ static const int MAX_DISTANCES[] = {1, 2, 5, 12, 19, 26};
  * Property-based testing of gridPathCells output
  */
 static void gridPathCells_assertions(H3Index start, H3Index end) {
-    int sz = H3_EXPORT(gridPathCellsSize)(start, end);
+    int64_t sz;
+    t_assertSuccess(H3_EXPORT(gridPathCellsSize)(start, end, &sz));
     t_assert(sz > 0, "got valid size");
     H3Index *line = calloc(sz, sizeof(H3Index));
 
-    int err = H3_EXPORT(gridPathCells)(start, end, line);
+    t_assertSuccess(H3_EXPORT(gridPathCells)(start, end, line));
 
-    t_assert(err == 0, "no error on line");
     t_assert(line[0] == start, "line starts with start index");
     t_assert(line[sz - 1] == end, "line ends with end index");
 
@@ -64,12 +64,13 @@ static void gridPathCells_assertions(H3Index start, H3Index end) {
  * Tests for invalid gridPathCells input
  */
 static void gridPathCells_invalid_assertions(H3Index start, H3Index end) {
-    int sz = H3_EXPORT(gridPathCellsSize)(start, end);
-    t_assert(sz < 0, "line size marked as invalid");
+    int64_t sz;
+    t_assert(H3_EXPORT(gridPathCellsSize)(start, end, &sz) != E_SUCCESS,
+             "line size marked as invalid");
 
     H3Index *line = {0};
-    int err = H3_EXPORT(gridPathCells)(start, end, line);
-    t_assert(err != 0, "line marked as invalid");
+    H3Error err = H3_EXPORT(gridPathCells)(start, end, line);
+    t_assert(err != E_SUCCESS, "line marked as invalid");
 }
 
 /**
@@ -93,8 +94,10 @@ static void gridPathCells_gridDisk_assertions(H3Index h3) {
         if (neighbors[i] == 0) {
             continue;
         }
-        int distance = H3_EXPORT(gridDistance)(h3, neighbors[i]);
-        if (distance >= 0) {
+        int64_t distance;
+        H3Error distanceError =
+            H3_EXPORT(gridDistance)(h3, neighbors[i], &distance);
+        if (distanceError == E_SUCCESS) {
             gridPathCells_assertions(h3, neighbors[i]);
         } else {
             gridPathCells_invalid_assertions(h3, neighbors[i]);

diff --git a/src/apps/testapps/testH3ToLocalIj.c b/src/apps/testapps/testH3ToLocalIj.c
@@ -100,7 +100,7 @@ SUITE(h3ToLocalIj) {
 
         for (int i = 0; i < numCoords; i++) {
             H3Index result;
-            const int err = H3_EXPORT(experimentalLocalIjToH3)(
+            const H3Error err = H3_EXPORT(experimentalLocalIjToH3)(
                 expected[0], &coords[i], &result);
             if (expected[i] == H3_NULL) {
                 t_assert(err != 0, "coordinates out of range");

diff --git a/src/h3lib/include/h3api.h.in b/src/h3lib/include/h3api.h.in
@@ -692,37 +692,40 @@ DECLSPEC int H3_EXPORT(isValidVertex)(H3Index vertex);
  * @{
  */
 /** @brief Returns grid distance between two indexes */
-DECLSPEC int H3_EXPORT(gridDistance)(H3Index origin, H3Index h3);
+DECLSPEC H3Error H3_EXPORT(gridDistance)(H3Index origin, H3Index h3,
+                                         int64_t *distance);
 /** @} */
 
 /** @defgroup gridPathCells gridPathCells
  * Functions for gridPathCells
  * @{
  */
 /** @brief Number of indexes in a line connecting two indexes */
-DECLSPEC int H3_EXPORT(gridPathCellsSize)(H3Index start, H3Index end);
+DECLSPEC H3Error H3_EXPORT(gridPathCellsSize)(H3Index start, H3Index end,
+                                              int64_t *size);
 
 /** @brief Line of h3 indexes connecting two indexes */
-DECLSPEC int H3_EXPORT(gridPathCells)(H3Index start, H3Index end, H3Index *out);
+DECLSPEC H3Error H3_EXPORT(gridPathCells)(H3Index start, H3Index end,
+                                          H3Index *out);
 /** @} */
 
 /** @defgroup experimentalH3ToLocalIj experimentalH3ToLocalIj
  * Functions for experimentalH3ToLocalIj
  * @{
  */
 /** @brief Returns two dimensional coordinates for the given index */
-DECLSPEC int H3_EXPORT(experimentalH3ToLocalIj)(H3Index origin, H3Index h3,
-                                                CoordIJ *out);
+DECLSPEC H3Error H3_EXPORT(experimentalH3ToLocalIj)(H3Index origin, H3Index h3,
+                                                    CoordIJ *out);
 /** @} */
 
 /** @defgroup experimentalLocalIjToH3 experimentalLocalIjToH3
  * Functions for experimentalLocalIjToH3
  * @{
  */
 /** @brief Returns index for the given two dimensional coordinates */
-DECLSPEC int H3_EXPORT(experimentalLocalIjToH3)(H3Index origin,
-                                                const CoordIJ *ij,
-                                                H3Index *out);
+DECLSPEC H3Error H3_EXPORT(experimentalLocalIjToH3)(H3Index origin,
+                                                    const CoordIJ *ij,
+                                                    H3Index *out);
 /** @} */
 
 #ifdef __cplusplus

diff --git a/src/h3lib/include/localij.h b/src/h3lib/include/localij.h
@@ -23,7 +23,7 @@
 #include "coordijk.h"
 #include "h3api.h"
 
-int h3ToLocalIjk(H3Index origin, H3Index h3, CoordIJK *out);
-int localIjkToH3(H3Index origin, const CoordIJK *ijk, H3Index *out);
+H3Error h3ToLocalIjk(H3Index origin, H3Index h3, CoordIJK *out);
+H3Error localIjkToH3(H3Index origin, const CoordIJK *ijk, H3Index *out);
 
 #endif