Skip to content

Commit 837fd2e

Browse files
authored
Make H3 CellBoundary immutable (#113792)
Small refactor that makes CellBoundary immutable.
1 parent 57955cb commit 837fd2e

File tree

8 files changed

+67
-37
lines changed

8 files changed

+67
-37
lines changed

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

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,36 +22,52 @@
2222
*/
2323
package org.elasticsearch.h3;
2424

25+
import java.util.Arrays;
26+
import java.util.Objects;
27+
2528
/**
2629
* cell boundary points as {@link LatLng}
2730
*/
2831
public final class CellBoundary {
29-
3032
/** Maximum number of cell boundary vertices; worst case is pentagon:
3133
* 5 original verts + 5 edge crossings
3234
*/
33-
private static final int MAX_CELL_BNDRY_VERTS = 10;
35+
static final int MAX_CELL_BNDRY_VERTS = 10;
3436
/** How many points it holds */
35-
private int numVertext;
37+
private final int numPoints;
3638
/** The actual points */
37-
private final LatLng[] points = new LatLng[MAX_CELL_BNDRY_VERTS];
38-
39-
CellBoundary() {}
39+
private final LatLng[] points;
4040

41-
void add(LatLng point) {
42-
points[numVertext++] = point;
41+
CellBoundary(LatLng[] points, int numPoints) {
42+
this.points = points;
43+
this.numPoints = numPoints;
4344
}
4445

4546
/** Number of points in this boundary */
4647
public int numPoints() {
47-
return numVertext;
48+
return numPoints;
4849
}
4950

5051
/** Return the point at the given position*/
5152
public LatLng getLatLon(int i) {
52-
if (i >= numVertext) {
53-
throw new IndexOutOfBoundsException();
54-
}
53+
assert i >= 0 && i < numPoints;
5554
return points[i];
5655
}
56+
57+
@Override
58+
public boolean equals(Object o) {
59+
if (this == o) {
60+
return true;
61+
}
62+
if (o == null || getClass() != o.getClass()) {
63+
return false;
64+
}
65+
final CellBoundary that = (CellBoundary) o;
66+
return numPoints == that.numPoints && Arrays.equals(points, that.points);
67+
}
68+
69+
@Override
70+
public int hashCode() {
71+
return Objects.hash(numPoints, Arrays.hashCode(points));
72+
}
5773
}

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,6 @@ final class Constants {
3434
* 2.0 * PI
3535
*/
3636
public static final double M_2PI = 2.0 * Math.PI;
37-
/**
38-
* max H3 resolution; H3 version 1 has 16 resolutions, numbered 0 through 15
39-
*/
40-
public static int MAX_H3_RES = 15;
4137
/**
4238
* The number of H3 base cells
4339
*/

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

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,8 @@ public CellBoundary faceIjkPentToCellBoundary(int res, int start, int length) {
439439
// convert each vertex to lat/lng
440440
// adjust the face of each vertex as appropriate and introduce
441441
// edge-crossing vertices as needed
442-
final CellBoundary boundary = new CellBoundary();
442+
final LatLng[] points = new LatLng[CellBoundary.MAX_CELL_BNDRY_VERTS];
443+
int numPoints = 0;
443444
final CoordIJK scratch = new CoordIJK(0, 0, 0);
444445
final FaceIJK fijk = new FaceIJK(this.face, scratch);
445446
final int[][] coord = isResolutionClassIII ? VERTEX_CLASSIII : VERTEX_CLASSII;
@@ -501,21 +502,19 @@ public CellBoundary faceIjkPentToCellBoundary(int res, int start, int length) {
501502

502503
// find the intersection and add the lat/lng point to the result
503504
final Vec2d inter = Vec2d.v2dIntersect(orig2d0, orig2d1, edge0, edge1);
504-
final LatLng point = inter.hex2dToGeo(fijkOrient.face, adjRes, true);
505-
boundary.add(point);
505+
points[numPoints++] = inter.hex2dToGeo(fijkOrient.face, adjRes, true);
506506
}
507507

508508
// convert vertex to lat/lng and add to the result
509509
// vert == start + NUM_PENT_VERTS is only used to test for possible
510510
// intersection on last edge
511511
if (vert < start + Constants.NUM_PENT_VERTS) {
512-
final LatLng point = fijk.coord.ijkToGeo(fijk.face, adjRes, true);
513-
boundary.add(point);
512+
points[numPoints++] = fijk.coord.ijkToGeo(fijk.face, adjRes, true);
514513
}
515514
lastFace = fijk.face;
516515
lastCoord.reset(fijk.coord.i, fijk.coord.j, fijk.coord.k);
517516
}
518-
return boundary;
517+
return new CellBoundary(points, numPoints);
519518
}
520519

521520
/**
@@ -547,7 +546,8 @@ public CellBoundary faceIjkToCellBoundary(final int res, final int start, final
547546
// convert each vertex to lat/lng
548547
// adjust the face of each vertex as appropriate and introduce
549548
// edge-crossing vertices as needed
550-
final CellBoundary boundary = new CellBoundary();
549+
final LatLng[] points = new LatLng[CellBoundary.MAX_CELL_BNDRY_VERTS];
550+
int numPoints = 0;
551551
final CoordIJK scratch1 = new CoordIJK(0, 0, 0);
552552
final FaceIJK fijk = new FaceIJK(this.face, scratch1);
553553
final CoordIJK scratch2 = isResolutionClassIII ? new CoordIJK(0, 0, 0) : null;
@@ -616,22 +616,20 @@ public CellBoundary faceIjkToCellBoundary(final int res, final int start, final
616616
*/
617617
final boolean isIntersectionAtVertex = orig2d0.numericallyIdentical(inter) || orig2d1.numericallyIdentical(inter);
618618
if (isIntersectionAtVertex == false) {
619-
final LatLng point = inter.hex2dToGeo(this.face, adjRes, true);
620-
boundary.add(point);
619+
points[numPoints++] = inter.hex2dToGeo(this.face, adjRes, true);
621620
}
622621
}
623622

624623
// convert vertex to lat/lng and add to the result
625624
// vert == start + NUM_HEX_VERTS is only used to test for possible
626625
// intersection on last edge
627626
if (vert < start + Constants.NUM_HEX_VERTS) {
628-
final LatLng point = fijk.coord.ijkToGeo(fijk.face, adjRes, true);
629-
boundary.add(point);
627+
points[numPoints++] = fijk.coord.ijkToGeo(fijk.face, adjRes, true);
630628
}
631629
lastFace = fijk.face;
632630
lastOverage = overage;
633631
}
634-
return boundary;
632+
return new CellBoundary(points, numPoints);
635633
}
636634

637635
/**

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,10 @@
3030
* Defines the public API of the H3 library.
3131
*/
3232
public final class H3 {
33-
34-
public static int MAX_H3_RES = Constants.MAX_H3_RES;
33+
/**
34+
* max H3 resolution; H3 version 1 has 16 resolutions, numbered 0 through 15
35+
*/
36+
public static int MAX_H3_RES = 15;
3537

3638
private static final long[] NORTH = new long[MAX_H3_RES + 1];
3739
private static final long[] SOUTH = new long[MAX_H3_RES + 1];
@@ -97,7 +99,7 @@ public static boolean h3IsValid(long h3) {
9799
}
98100

99101
int res = H3Index.H3_get_resolution(h3);
100-
if (res < 0 || res > Constants.MAX_H3_RES) { // LCOV_EXCL_BR_LINE
102+
if (res < 0 || res > MAX_H3_RES) { // LCOV_EXCL_BR_LINE
101103
// Resolutions less than zero can not be represented in an index
102104
return false;
103105
}
@@ -118,7 +120,7 @@ public static boolean h3IsValid(long h3) {
118120
}
119121
}
120122

121-
for (int r = res + 1; r <= Constants.MAX_H3_RES; r++) {
123+
for (int r = res + 1; r <= MAX_H3_RES; r++) {
122124
int digit = H3Index.H3_get_index_digit(h3, r);
123125
if (digit != CoordIJK.Direction.INVALID_DIGIT.digit()) {
124126
return false;
@@ -601,7 +603,7 @@ private static String[] h3ToStringList(long[] h3s) {
601603
* @throws IllegalArgumentException <code>res</code> is not a valid H3 resolution.
602604
*/
603605
private static void checkResolution(int res) {
604-
if (res < 0 || res > Constants.MAX_H3_RES) {
606+
if (res < 0 || res > MAX_H3_RES) {
605607
throw new IllegalArgumentException("resolution [" + res + "] is out of range (must be 0 <= res <= 15)");
606608
}
607609
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,14 +160,14 @@ public static int H3_get_resolution(long h3) {
160160
* Gets the resolution res integer digit (0-7) of h3.
161161
*/
162162
public static int H3_get_index_digit(long h3, int res) {
163-
return ((int) ((((h3) >> ((Constants.MAX_H3_RES - (res)) * H3_PER_DIGIT_OFFSET)) & H3_DIGIT_MASK)));
163+
return ((int) ((((h3) >> ((H3.MAX_H3_RES - (res)) * H3_PER_DIGIT_OFFSET)) & H3_DIGIT_MASK)));
164164
}
165165

166166
/**
167167
* Sets the resolution res digit of h3 to the integer digit (0-7)
168168
*/
169169
public static long H3_set_index_digit(long h3, int res, long digit) {
170-
int x = (Constants.MAX_H3_RES - res) * H3_PER_DIGIT_OFFSET;
170+
int x = (H3.MAX_H3_RES - res) * H3_PER_DIGIT_OFFSET;
171171
return (((h3) & ~((H3_DIGIT_MASK << (x)))) | (((digit)) << x));
172172
}
173173

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,4 +218,22 @@ private boolean isSharedBoundary(int clon1, int clat1, int clon2, int clat2, Cel
218218
}
219219
return false;
220220
}
221+
222+
public void testEqualsAndHashCode() {
223+
final long h3 = H3.geoToH3(GeoTestUtil.nextLatitude(), GeoTestUtil.nextLongitude(), randomIntBetween(0, 15));
224+
final CellBoundary boundary1 = H3.h3ToGeoBoundary(h3);
225+
final CellBoundary boundary2 = H3.h3ToGeoBoundary(h3);
226+
assertEquals(boundary1, boundary2);
227+
assertEquals(boundary1.hashCode(), boundary2.hashCode());
228+
229+
final long otherH3 = H3.geoToH3(GeoTestUtil.nextLatitude(), GeoTestUtil.nextLongitude(), randomIntBetween(0, 15));
230+
final CellBoundary otherCellBoundary = H3.h3ToGeoBoundary(otherH3);
231+
if (otherH3 != h3) {
232+
assertNotEquals(boundary1, otherCellBoundary);
233+
assertNotEquals(boundary1.hashCode(), otherCellBoundary.hashCode());
234+
} else {
235+
assertEquals(boundary1, otherCellBoundary);
236+
assertEquals(boundary1.hashCode(), otherCellBoundary.hashCode());
237+
}
238+
}
221239
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public void testRandomPoints() {
3838

3939
private void testPoint(double lat, double lon) {
4040
GeoPoint point = new GeoPoint(PlanetModel.SPHERE, Math.toRadians(lat), Math.toRadians(lon));
41-
for (int res = 0; res < Constants.MAX_H3_RES; res++) {
41+
for (int res = 0; res < H3.MAX_H3_RES; res++) {
4242
String h3Address = H3.geoToH3Address(lat, lon, res);
4343
assertEquals(res, H3.getResolution(h3Address));
4444
GeoPolygon polygon = getGeoPolygon(h3Address);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public void testHexRing() {
3838
for (int i = 0; i < 500; i++) {
3939
double lat = GeoTestUtil.nextLatitude();
4040
double lon = GeoTestUtil.nextLongitude();
41-
for (int res = 0; res <= Constants.MAX_H3_RES; res++) {
41+
for (int res = 0; res <= H3.MAX_H3_RES; res++) {
4242
String origin = H3.geoToH3Address(lat, lon, res);
4343
assertFalse(H3.areNeighborCells(origin, origin));
4444
String[] ring = H3.hexRing(origin);

0 commit comments

Comments
 (0)