Make H3 CellBoundary immutable (#113792) (#114204)

Small refactor that makes CellBoundary immutable.

libs/h3/src/main/java/org/elasticsearch/h3/CellBoundary.java

@@ -22,36 +22,52 @@
  */
 package org.elasticsearch.h3;
 
+import java.util.Arrays;
+import java.util.Objects;
+
 /**
  * cell boundary points as {@link LatLng}
  */
 public final class CellBoundary {
-
     /** Maximum number of cell boundary vertices; worst case is pentagon:
      *  5 original verts + 5 edge crossings
      */
-    private static final int MAX_CELL_BNDRY_VERTS = 10;
+    static final int MAX_CELL_BNDRY_VERTS = 10;
     /** How many points it holds */
-    private int numVertext;
+    private final int numPoints;
     /** The actual points */
-    private final LatLng[] points = new LatLng[MAX_CELL_BNDRY_VERTS];
-
-    CellBoundary() {}
+    private final LatLng[] points;
 
-    void add(LatLng point) {
-        points[numVertext++] = point;
+    CellBoundary(LatLng[] points, int numPoints) {
+        this.points = points;
+        this.numPoints = numPoints;
     }
 
     /** Number of points in this boundary */
     public int numPoints() {
-        return numVertext;
+        return numPoints;
     }
 
     /** Return the point at the given position*/
     public LatLng getLatLon(int i) {
-        if (i >= numVertext) {
-            throw new IndexOutOfBoundsException();
-        }
+        assert i >= 0 && i < numPoints;
         return points[i];
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+        final CellBoundary that = (CellBoundary) o;
+        return numPoints == that.numPoints && Arrays.equals(points, that.points);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(numPoints, Arrays.hashCode(points));
+    }
 }

libs/h3/src/main/java/org/elasticsearch/h3/Constants.java

@@ -34,10 +34,6 @@ final class Constants {
      * 2.0 * PI
      */
     public static final double M_2PI = 2.0 * Math.PI;
-    /**
-     * max H3 resolution; H3 version 1 has 16 resolutions, numbered 0 through 15
-     */
-    public static int MAX_H3_RES = 15;
     /**
      * The number of H3 base cells
      */

libs/h3/src/main/java/org/elasticsearch/h3/FaceIJK.java

@@ -439,7 +439,8 @@ final class FaceIJK {
         // convert each vertex to lat/lng
         // adjust the face of each vertex as appropriate and introduce
         // edge-crossing vertices as needed
-        final CellBoundary boundary = new CellBoundary();
+        final LatLng[] points = new LatLng[CellBoundary.MAX_CELL_BNDRY_VERTS];
+        int numPoints = 0;
         final CoordIJK scratch = new CoordIJK(0, 0, 0);
         final FaceIJK fijk = new FaceIJK(this.face, scratch);
         final int[][] coord = isResolutionClassIII ? VERTEX_CLASSIII : VERTEX_CLASSII;
@@ -501,21 +502,19 @@ final class FaceIJK {
 
                 // find the intersection and add the lat/lng point to the result
                 final Vec2d inter = Vec2d.v2dIntersect(orig2d0, orig2d1, edge0, edge1);
-                final LatLng point = inter.hex2dToGeo(fijkOrient.face, adjRes, true);
-                boundary.add(point);
+                points[numPoints++] = inter.hex2dToGeo(fijkOrient.face, adjRes, true);
             }
 
             // convert vertex to lat/lng and add to the result
             // vert == start + NUM_PENT_VERTS is only used to test for possible
             // intersection on last edge
             if (vert < start + Constants.NUM_PENT_VERTS) {
-                final LatLng point = fijk.coord.ijkToGeo(fijk.face, adjRes, true);
-                boundary.add(point);
+                points[numPoints++] = fijk.coord.ijkToGeo(fijk.face, adjRes, true);
             }
             lastFace = fijk.face;
             lastCoord.reset(fijk.coord.i, fijk.coord.j, fijk.coord.k);
         }
-        return boundary;
+        return new CellBoundary(points, numPoints);
     }
 
     /**
@@ -547,7 +546,8 @@ final class FaceIJK {
         // convert each vertex to lat/lng
         // adjust the face of each vertex as appropriate and introduce
         // edge-crossing vertices as needed
-        final CellBoundary boundary = new CellBoundary();
+        final LatLng[] points = new LatLng[CellBoundary.MAX_CELL_BNDRY_VERTS];
+        int numPoints = 0;
         final CoordIJK scratch1 = new CoordIJK(0, 0, 0);
         final FaceIJK fijk = new FaceIJK(this.face, scratch1);
         final CoordIJK scratch2 = isResolutionClassIII ? new CoordIJK(0, 0, 0) : null;
@@ -616,8 +616,7 @@ final class FaceIJK {
                 */
                 final boolean isIntersectionAtVertex = orig2d0.numericallyIdentical(inter) || orig2d1.numericallyIdentical(inter);
                 if (isIntersectionAtVertex == false) {
-                    final LatLng point = inter.hex2dToGeo(this.face, adjRes, true);
-                    boundary.add(point);
+                    points[numPoints++] = inter.hex2dToGeo(this.face, adjRes, true);
                 }
             }
 
@@ -625,13 +624,12 @@ final class FaceIJK {
             // vert == start + NUM_HEX_VERTS is only used to test for possible
             // intersection on last edge
             if (vert < start + Constants.NUM_HEX_VERTS) {
-                final LatLng point = fijk.coord.ijkToGeo(fijk.face, adjRes, true);
-                boundary.add(point);
+                points[numPoints++] = fijk.coord.ijkToGeo(fijk.face, adjRes, true);
             }
             lastFace = fijk.face;
             lastOverage = overage;
         }
-        return boundary;
+        return new CellBoundary(points, numPoints);
     }
 
     /**

libs/h3/src/main/java/org/elasticsearch/h3/H3.java

@@ -30,8 +30,10 @@ import static java.lang.Math.toRadians;
  * Defines the public API of the H3 library.
  */
 public final class H3 {
-
-    public static int MAX_H3_RES = Constants.MAX_H3_RES;
+    /**
+     * max H3 resolution; H3 version 1 has 16 resolutions, numbered 0 through 15
+     */
+    public static int MAX_H3_RES = 15;
 
     private static final long[] NORTH = new long[MAX_H3_RES + 1];
     private static final long[] SOUTH = new long[MAX_H3_RES + 1];
@@ -97,7 +99,7 @@ public final class H3 {
         }
 
         int res = H3Index.H3_get_resolution(h3);
-        if (res < 0 || res > Constants.MAX_H3_RES) {  // LCOV_EXCL_BR_LINE
+        if (res < 0 || res > MAX_H3_RES) {  // LCOV_EXCL_BR_LINE
             // Resolutions less than zero can not be represented in an index
             return false;
         }
@@ -118,7 +120,7 @@ public final class H3 {
             }
         }
 
-        for (int r = res + 1; r <= Constants.MAX_H3_RES; r++) {
+        for (int r = res + 1; r <= MAX_H3_RES; r++) {
             int digit = H3Index.H3_get_index_digit(h3, r);
             if (digit != CoordIJK.Direction.INVALID_DIGIT.digit()) {
                 return false;
@@ -601,7 +603,7 @@ public final class H3 {
      * @throws IllegalArgumentException <code>res</code> is not a valid H3 resolution.
      */
     private static void checkResolution(int res) {
-        if (res < 0 || res > Constants.MAX_H3_RES) {
+        if (res < 0 || res > MAX_H3_RES) {
             throw new IllegalArgumentException("resolution [" + res + "]  is out of range (must be 0 <= res <= 15)");
         }
     }

libs/h3/src/main/java/org/elasticsearch/h3/H3Index.java

@@ -160,14 +160,14 @@ final class H3Index {
      * Gets the resolution res integer digit (0-7) of h3.
      */
     public static int H3_get_index_digit(long h3, int res) {
-        return ((int) ((((h3) >> ((Constants.MAX_H3_RES - (res)) * H3_PER_DIGIT_OFFSET)) & H3_DIGIT_MASK)));
+        return ((int) ((((h3) >> ((H3.MAX_H3_RES - (res)) * H3_PER_DIGIT_OFFSET)) & H3_DIGIT_MASK)));
     }
 
     /**
      * Sets the resolution res digit of h3 to the integer digit (0-7)
      */
     public static long H3_set_index_digit(long h3, int res, long digit) {
-        int x = (Constants.MAX_H3_RES - res) * H3_PER_DIGIT_OFFSET;
+        int x = (H3.MAX_H3_RES - res) * H3_PER_DIGIT_OFFSET;
         return (((h3) & ~((H3_DIGIT_MASK << (x)))) | (((digit)) << x));
     }
 

libs/h3/src/test/java/org/elasticsearch/h3/CellBoundaryTests.java

@@ -218,4 +218,22 @@ public class CellBoundaryTests extends ESTestCase {
         }
         return false;
     }
+
+    public void testEqualsAndHashCode() {
+        final long h3 = H3.geoToH3(GeoTestUtil.nextLatitude(), GeoTestUtil.nextLongitude(), randomIntBetween(0, 15));
+        final CellBoundary boundary1 = H3.h3ToGeoBoundary(h3);
+        final CellBoundary boundary2 = H3.h3ToGeoBoundary(h3);
+        assertEquals(boundary1, boundary2);
+        assertEquals(boundary1.hashCode(), boundary2.hashCode());
+
+        final long otherH3 = H3.geoToH3(GeoTestUtil.nextLatitude(), GeoTestUtil.nextLongitude(), randomIntBetween(0, 15));
+        final CellBoundary otherCellBoundary = H3.h3ToGeoBoundary(otherH3);
+        if (otherH3 != h3) {
+            assertNotEquals(boundary1, otherCellBoundary);
+            assertNotEquals(boundary1.hashCode(), otherCellBoundary.hashCode());
+        } else {
+            assertEquals(boundary1, otherCellBoundary);
+            assertEquals(boundary1.hashCode(), otherCellBoundary.hashCode());
+        }
+    }
 }

libs/h3/src/test/java/org/elasticsearch/h3/GeoToH3Tests.java

@@ -38,7 +38,7 @@ public class GeoToH3Tests extends ESTestCase {
 
     private void testPoint(double lat, double lon) {
         GeoPoint point = new GeoPoint(PlanetModel.SPHERE, Math.toRadians(lat), Math.toRadians(lon));
-        for (int res = 0; res < Constants.MAX_H3_RES; res++) {
+        for (int res = 0; res < H3.MAX_H3_RES; res++) {
             String h3Address = H3.geoToH3Address(lat, lon, res);
             assertEquals(res, H3.getResolution(h3Address));
             GeoPolygon polygon = getGeoPolygon(h3Address);

libs/h3/src/test/java/org/elasticsearch/h3/HexRingTests.java

@@ -38,7 +38,7 @@ public class HexRingTests extends ESTestCase {
         for (int i = 0; i < 500; i++) {
             double lat = GeoTestUtil.nextLatitude();
             double lon = GeoTestUtil.nextLongitude();
-            for (int res = 0; res <= Constants.MAX_H3_RES; res++) {
+            for (int res = 0; res <= H3.MAX_H3_RES; res++) {
                 String origin = H3.geoToH3Address(lat, lon, res);
                 assertFalse(H3.areNeighborCells(origin, origin));
                 String[] ring = H3.hexRing(origin);