A more complex CL to avoid exposing the 'bounds' arrays from several of the

geometry class (paving the way for them to be made immutable later).
-------------
Created by MOE: http://code.google.com/p/moe-java
MOE_MIGRATED_REVID=24952612

10 files changed, 87 insertions, 106 deletions

diff --git a/src/com/google/common/geometry/R1Interval.java b/src/com/google/common/geometry/R1Interval.java
index e8edbe4..5b912ec 100644
--- a/src/com/google/common/geometry/R1Interval.java
+++ b/src/com/google/common/geometry/R1Interval.java
@@ -22,13 +22,16 @@ package com.google.common.geometry;
  *
  */
 
-public strictfp class R1Interval {
-  private final double[] bounds = new double[2];
+public final strictfp class R1Interval {
+
+  // TODO(dbeaumont): Make these final and make this class fully immutable.
+  private double lo;
+  private double hi;
 
   /** Interval constructor. If lo > hi, the interval is empty. */
   public R1Interval(double lo, double hi) {
-    bounds[0] = lo;
-    bounds[1] = hi;
+    this.lo = lo;
+    this.hi = hi;
   }
 
   /**
@@ -60,27 +63,19 @@ public strictfp class R1Interval {
   }
 
   public double lo() {
-    return bounds[0];
+    return lo;
   }
 
   public double hi() {
-    return bounds[1];
-  }
-
-  public double bound(int i) {
-    return bounds[i];
-  }
-
-  public double[] bounds() {
-    return bounds;
+    return hi;
   }
 
   public void setLo(double p) {
-    bounds[0] = p;
+    this.lo = p;
   }
 
   public void setHi(double p) {
-    bounds[1] = p;
+    this.hi = p;
   }
 
   /**
@@ -98,7 +93,6 @@ public strictfp class R1Interval {
     return 0.5 * (lo() + hi());
   }
 
-
   /**
    * Return the length of the interval. The length of an empty interval is
    * negative.
@@ -107,7 +101,6 @@ public strictfp class R1Interval {
     return hi() - lo();
   }
 
-
   public boolean contains(double p) {
     return p >= lo() && p <= hi();
   }
@@ -221,8 +214,8 @@ public strictfp class R1Interval {
     }
 
     long value = 17;
-    value = 37 * value + Double.doubleToLongBits(bounds[0]);
-    value = 37 * value + Double.doubleToLongBits(bounds[1]);
+    value = 37 * value + Double.doubleToLongBits(lo);
+    value = 37 * value + Double.doubleToLongBits(hi);
     return (int) (value ^ (value >>> 32));
   }
 
diff --git a/src/com/google/common/geometry/R2Vector.java b/src/com/google/common/geometry/R2Vector.java
index 70ca580..2198d1f 100644
--- a/src/com/google/common/geometry/R2Vector.java
+++ b/src/com/google/common/geometry/R2Vector.java
@@ -21,9 +21,13 @@ package com.google.common.geometry;
  * norm, comparison etc.
  *
  */
-public strictfp class R2Vector {
-  double x;
-  double y;
+public final strictfp class R2Vector {
+  final double x;
+  final double y;
+
+  public R2Vector() {
+    this(0, 0);
+  }
 
   public R2Vector(double x, double y) {
     this.x = x;
@@ -38,9 +42,6 @@ public strictfp class R2Vector {
     y = coord[1];
   }
 
-  public R2Vector() {
-  }
-
   public double get(int index) {
     if (index > 1) {
       throw new ArrayIndexOutOfBoundsException(index);
@@ -57,11 +58,11 @@ public strictfp class R2Vector {
   }
 
   public double norm2() {
-    return x * x + y * y;
+    return (x * x) + (y * y);
   }
 
   public static double dotProd(final R2Vector p1, final R2Vector p2) {
-    return p1.x * p2.x + p1.y * p2.y;
+    return (p1.x * p2.x) + (p1.y * p2.y);
   }
 
   public double dotProd(R2Vector that) {
diff --git a/src/com/google/common/geometry/S1Interval.java b/src/com/google/common/geometry/S1Interval.java
index fa6f1ad..eafca63 100644
--- a/src/com/google/common/geometry/S1Interval.java
+++ b/src/com/google/common/geometry/S1Interval.java
@@ -36,9 +36,11 @@ package com.google.common.geometry;
  *
  */
 
-public strictfp class S1Interval implements Cloneable {
+public final strictfp class S1Interval implements Cloneable {
 
-  private final double[] bounds = new double[2];
+  // TODO(dbeaumont): Make this class immutable and fix callers.
+  private double lo;
+  private double hi;
 
   /**
    * Both endpoints must be in the range -Pi to Pi inclusive. The value -Pi is
@@ -50,10 +52,12 @@ public strictfp class S1Interval implements Cloneable {
 
   /**
    * Copy constructor. Assumes that the given interval is valid.
+   *
+   * TODO(dbeaumont): Make this class immutable and remove this method.
    */
   public S1Interval(S1Interval interval) {
-    bounds[0] = interval.bounds[0];
-    bounds[1] = interval.bounds[1];
+    this.lo = interval.lo;
+    this.hi = interval.hi;
   }
 
   /**
@@ -61,21 +65,20 @@ public strictfp class S1Interval implements Cloneable {
    * range, i.e. normalization from -Pi to Pi is already done.
    */
   private S1Interval(double lo, double hi, boolean checked) {
-    bounds[0] = lo;
-    bounds[1] = hi;
-
+    double newLo = lo;
+    double newHi = hi;
     if (!checked) {
       if (lo == -S2.M_PI && hi != S2.M_PI) {
-        setLo(S2.M_PI);
+        newLo = S2.M_PI;
       }
       if (hi == -S2.M_PI && lo != S2.M_PI) {
-        setHi(S2.M_PI);
+        newHi = S2.M_PI;
       }
     }
-    // assert (isValid());
+    this.lo = newLo;
+    this.hi = newHi;
   }
 
-
   public static S1Interval empty() {
     return new S1Interval(S2.M_PI, -S2.M_PI, true);
   }
@@ -112,30 +115,21 @@ public strictfp class S1Interval implements Cloneable {
     }
   }
 
-
   public double lo() {
-    return bounds[0];
+    return lo;
   }
 
   public double hi() {
-    return bounds[1];
+    return hi;
   }
 
-  public double bound(int i) {
-    return bounds[i];
-  }
-
-  public double[] bounds() {
-    return bounds;
-  }
-
-  public void setLo(double p) {
-    bounds[0] = p;
+  public void setLo(double lo) {
+    this.lo = lo;
     // assert (isValid());
   }
 
-  public void setHi(double p) {
-    bounds[1] = p;
+  public void setHi(double hi) {
+    this.hi = hi;
     // assert (isValid());
   }
 
@@ -148,7 +142,6 @@ public strictfp class S1Interval implements Cloneable {
         && !(lo() == -S2.M_PI && hi() != S2.M_PI) && !(hi() == -S2.M_PI && lo() != S2.M_PI));
   }
 
-
   /** Return true if the interval contains all points on the unit circle. */
   public boolean isFull() {
     return hi() - lo() == 2 * S2.M_PI;
@@ -166,7 +159,6 @@ public strictfp class S1Interval implements Cloneable {
     return lo() > hi();
   }
 
-
   /**
    * Return the midpoint of the interval. For full and empty intervals, the
    * result is arbitrary.
@@ -194,7 +186,6 @@ public strictfp class S1Interval implements Cloneable {
     return (length > 0) ? length : -1;
   }
 
-
   /**
    * Return the complement of the interior of the interval. An interval and its
    * complement have the same boundary but do not share any interior values. The
@@ -221,7 +212,6 @@ public strictfp class S1Interval implements Cloneable {
     return fastContains(p);
   }
 
-
   /**
    * Return true if the interval (which is closed) contains the point 'p'. Skips
    * the normalization of 'p' from -Pi to Pi.
@@ -235,7 +225,6 @@ public strictfp class S1Interval implements Cloneable {
     }
   }
 
-
   /** Return true if the interior of the interval contains the point 'p'. */
   public boolean interiorContains(double p) {
     // Works for empty, full, and singleton intervals.
@@ -251,7 +240,6 @@ public strictfp class S1Interval implements Cloneable {
     }
   }
 
-
   /**
    * Return true if the interval contains the given interval 'y'. Works for
    * empty, full, and singleton intervals.
@@ -332,7 +320,6 @@ public strictfp class S1Interval implements Cloneable {
     }
   }
 
-
   /**
    * Expand the interval by the minimum amount necessary so that it contains the
    * given point "p" (an angle in the range [-Pi, Pi]).
@@ -388,7 +375,6 @@ public strictfp class S1Interval implements Cloneable {
     return result;
   }
 
-
   /**
    * Return the smallest interval that contains this interval and the given
    * interval "y".
@@ -432,7 +418,6 @@ public strictfp class S1Interval implements Cloneable {
     }
   }
 
-
   /**
    * Return the smallest interval that contains the intersection of this
    * interval with "y". Note that the region of intersection may consist of two
@@ -472,7 +457,6 @@ public strictfp class S1Interval implements Cloneable {
     return empty();
   }
 
-
   /**
    * Return true if the length of the symmetric difference between the two
    * intervals is at most the given tolerance.
diff --git a/src/com/google/common/geometry/S2Cell.java b/src/com/google/common/geometry/S2Cell.java
index 35f70ea..26c7af1 100644
--- a/src/com/google/common/geometry/S2Cell.java
+++ b/src/com/google/common/geometry/S2Cell.java
@@ -23,11 +23,10 @@ package com.google.common.geometry;
  *
  */
 
-public strictfp class S2Cell implements S2Region {
+public final strictfp class S2Cell implements S2Region {
 
   private static final int MAX_CELL_SIZE = 1 << S2CellId.MAX_LEVEL;
 
-  // This structure occupies 44 bytes plus one pointer for the vtable.
   byte face;
   byte level;
   byte orientation;
@@ -182,17 +181,17 @@ public strictfp class S2Cell implements S2Region {
   public R2Vector getCenterUV() {
     MutableInteger i = new MutableInteger(0);
     MutableInteger j = new MutableInteger(0);
-    R2Vector centerUv = new R2Vector();
     cellId.toFaceIJOrientation(i, j, null);
     int cellSize = 1 << (S2CellId.MAX_LEVEL - level);
 
-    int sij = (i.intValue() & -cellSize) * 2 + cellSize - MAX_CELL_SIZE;
-    centerUv.x = S2Projections.stToUV((1.0 / MAX_CELL_SIZE) * sij);
+    // TODO(dbeaumont): Figure out a better naming of the variables here (and elsewhere).
+    int si = (i.intValue() & -cellSize) * 2 + cellSize - MAX_CELL_SIZE;
+    double x = S2Projections.stToUV((1.0 / MAX_CELL_SIZE) * si);
 
-    sij = (j.intValue() & -cellSize) * 2 + cellSize - MAX_CELL_SIZE;
-    centerUv.y = S2Projections.stToUV((1.0 / MAX_CELL_SIZE) * sij);
+    int sj = (j.intValue() & -cellSize) * 2 + cellSize - MAX_CELL_SIZE;
+    double y = S2Projections.stToUV((1.0 / MAX_CELL_SIZE) * sj);
 
-    return centerUv;
+    return new R2Vector(x, y);
   }
 
   /**
@@ -365,8 +364,8 @@ public strictfp class S2Cell implements S2Region {
     // We can't just call XYZtoFaceUV, because for points that lie on the
     // boundary between two faces (i.e. u or v is +1/-1) we need to return
     // true for both adjacent cells.
-    R2Vector uvPoint = new R2Vector();
-    if (!S2Projections.faceXyzToUv(face, p, uvPoint)) {
+    R2Vector uvPoint = S2Projections.faceXyzToUv(face, p);
+    if (uvPoint == null) {
       return false;
     }
     return (uvPoint.x >= uv[0][0] && uvPoint.x <= uv[0][1] && uvPoint.y >= uv[1][0]
diff --git a/src/com/google/common/geometry/S2CellId.java b/src/com/google/common/geometry/S2CellId.java
index 4e5f1a3..4de0a59 100644
--- a/src/com/google/common/geometry/S2CellId.java
+++ b/src/com/google/common/geometry/S2CellId.java
@@ -48,7 +48,7 @@ import java.util.Locale;
  *
  *
  */
-public strictfp class S2CellId implements Comparable<S2CellId> {
+public final strictfp class S2CellId implements Comparable<S2CellId> {
 
   // Although only 60 bits are needed to represent the index of a leaf
   // cell, we need an extra bit in order to represent the position of
@@ -146,8 +146,8 @@ public strictfp class S2CellId implements Comparable<S2CellId> {
    * necessarily unit length).
    */
   public static S2CellId fromPoint(S2Point p) {
-    R2Vector uv = new R2Vector();
-    int face = S2Projections.xyzToFaceUV(p, uv);
+    int face = S2Projections.xyzToFace(p);
+    R2Vector uv = S2Projections.validFaceXyzToUv(face, p);
     int i = stToIJ(S2Projections.uvToST(uv.x));
     int j = stToIJ(S2Projections.uvToST(uv.y));
     return fromFaceIJ(face, i, j);
@@ -863,8 +863,9 @@ public strictfp class S2CellId implements Comparable<S2CellId> {
 
     // Find the leaf cell coordinates on the adjacent face, and convert
     // them to a cell id at the appropriate level.
-    R2Vector st = new R2Vector();
-    face = S2Projections.xyzToFaceUV(S2Projections.faceUvToXyz(face, s, t), st);
+    S2Point p = S2Projections.faceUvToXyz(face, s, t);
+    face = S2Projections.xyzToFace(p);
+    R2Vector st = S2Projections.validFaceXyzToUv(face, p);
     return fromFaceIJ(face, stToIJ(st.x), stToIJ(st.y));
   }
 
@@ -960,5 +961,4 @@ public strictfp class S2CellId implements Comparable<S2CellId> {
     return unsignedLongLessThan(this.id, that.id) ? -1 :
         unsignedLongGreaterThan(this.id, that.id) ? 1 : 0;
   }
-
 }
diff --git a/src/com/google/common/geometry/S2LatLngRect.java b/src/com/google/common/geometry/S2LatLngRect.java
index cb9ad0c..fae637b 100644
--- a/src/com/google/common/geometry/S2LatLngRect.java
+++ b/src/com/google/common/geometry/S2LatLngRect.java
@@ -23,7 +23,7 @@ import com.google.common.base.Preconditions;
  *
  */
 
-public strictfp class S2LatLngRect implements S2Region {
+public final strictfp class S2LatLngRect implements S2Region {
 
   private final R1Interval lat;
   private final S1Interval lng;
@@ -196,9 +196,19 @@ public strictfp class S2LatLngRect implements S2Region {
 
   /** Return the k-th vertex of the rectangle (k = 0,1,2,3) in CCW order. */
   public S2LatLng getVertex(int k) {
-    // Twiddle bits to return the points in CCW order (SW, SE, NE, NW).
-    return S2LatLng.fromRadians(lat.bound(k >> 1), lng.bound((k >> 1)
-      ^ (k & 1)));
+    // Return the points in CCW order (SW, SE, NE, NW).
+    switch (k) {
+      case 0:
+        return S2LatLng.fromRadians(lat.lo(), lng.lo());
+      case 1:
+        return S2LatLng.fromRadians(lat.lo(), lng.hi());
+      case 2:
+        return S2LatLng.fromRadians(lat.hi(), lng.hi());
+      case 3:
+        return S2LatLng.fromRadians(lat.hi(), lng.lo());
+      default:
+        throw new IllegalArgumentException("Invalid vertex index.");
+    }
   }
 
   /**
diff --git a/src/com/google/common/geometry/S2Projections.java b/src/com/google/common/geometry/S2Projections.java
index 7032799..0f366b1 100644
--- a/src/com/google/common/geometry/S2Projections.java
+++ b/src/com/google/common/geometry/S2Projections.java
@@ -69,7 +69,7 @@ import com.google.common.geometry.S2.Metric;
  *
  */
 
-public strictfp class S2Projections {
+public final strictfp class S2Projections {
   public enum Projections {
     S2_LINEAR_PROJECTION, S2_TAN_PROJECTION, S2_QUADRATIC_PROJECTION
   }
@@ -333,31 +333,25 @@ public strictfp class S2Projections {
     return new R2Vector(pu, pv);
   }
 
-  public static int xyzToFaceUV(S2Point p, R2Vector uv) {
+  public static int xyzToFace(S2Point p) {
     int face = p.largestAbsComponent();
     if (p.get(face) < 0) {
       face += 3;
     }
-    R2Vector point = validFaceXyzToUv(face, p);
-    uv.x = point.x;
-    uv.y = point.y;
     return face;
   }
 
-  public static boolean faceXyzToUv(int face, S2Point p, R2Vector uv) {
+  public static R2Vector faceXyzToUv(int face, S2Point p) {
     if (face < 3) {
       if (p.get(face) <= 0) {
-        return false;
+        return null;
       }
     } else {
       if (p.get(face - 3) >= 0) {
-        return false;
+        return null;
       }
     }
-    R2Vector point = validFaceXyzToUv(face, p);
-    uv.x = point.x;
-    uv.y = point.y;
-    return true;
+    return validFaceXyzToUv(face, p);
   }
 
   public static S2Point getUNorm(int face, double u) {
diff --git a/tests/com/google/common/geometry/R1IntervalTest.java b/tests/com/google/common/geometry/R1IntervalTest.java
index 0fe87ea..b4c2bba 100644
--- a/tests/com/google/common/geometry/R1IntervalTest.java
+++ b/tests/com/google/common/geometry/R1IntervalTest.java
@@ -40,8 +40,8 @@ public strictfp class R1IntervalTest extends GeometryTestCase {
     R1Interval negunit = new R1Interval(-1, 0);
     assertEquals(unit.lo(), 0.0);
     assertEquals(unit.hi(), 1.0);
-    assertEquals(negunit.bound(0), -1.0);
-    assertEquals(negunit.bound(1), 0.0);
+    assertEquals(negunit.lo(), -1.0);
+    assertEquals(negunit.hi(), 0.0);
     R1Interval ten = new R1Interval(0, 0);
     ten.setHi(10);
     assertEquals(ten.hi(), 10.0);
diff --git a/tests/com/google/common/geometry/S1IntervalTest.java b/tests/com/google/common/geometry/S1IntervalTest.java
index 146ec74..2215104 100644
--- a/tests/com/google/common/geometry/S1IntervalTest.java
+++ b/tests/com/google/common/geometry/S1IntervalTest.java
@@ -15,7 +15,6 @@
  */
 package com.google.common.geometry;
 
-import java.util.Arrays;
 
 public strictfp class S1IntervalTest extends GeometryTestCase {
 
@@ -33,15 +32,15 @@ public strictfp class S1IntervalTest extends GeometryTestCase {
 
     // bounds() returns a const reference to a member variable, so we need to
     // make a copy when invoking it on a temporary object.
-    assertTrue(Arrays.equals(x.union(y).bounds(), expectedUnion.bounds()));
-    assertTrue(Arrays.equals(x.intersection(y).bounds(), expectedIntersection.bounds()));
+    assertEquals(expectedUnion, x.union(y));
+    assertEquals(expectedIntersection, x.intersection(y));
 
     assertEquals(x.contains(y), x.union(y) == x);
     assertEquals(x.intersects(y), !x.intersection(y).isEmpty());
 
     if (y.lo() == y.hi()) {
       S1Interval r = x.addPoint(y.lo());
-      assertTrue(Arrays.equals(r.bounds(), expectedUnion.bounds()));
+      assertEquals(expectedUnion, r);
     }
   }
 
@@ -57,8 +56,8 @@ public strictfp class S1IntervalTest extends GeometryTestCase {
     assertEquals(quad12.lo(), 0.0);
     assertEquals(quad12.hi(), S2.M_PI);
     S1Interval quad34 = new S1Interval(-S2.M_PI, 0);
-    assertEquals(quad34.bound(0), S2.M_PI);
-    assertEquals(quad34.bound(1), 0.0);
+    assertEquals(quad34.lo(), S2.M_PI);
+    assertEquals(quad34.hi(), 0.0);
     S1Interval pi = new S1Interval(S2.M_PI, S2.M_PI);
     assertEquals(pi.lo(), S2.M_PI);
     assertEquals(pi.hi(), S2.M_PI);
diff --git a/tests/com/google/common/geometry/S2CellIdTest.java b/tests/com/google/common/geometry/S2CellIdTest.java
index ce089db..456c542 100644
--- a/tests/com/google/common/geometry/S2CellIdTest.java
+++ b/tests/com/google/common/geometry/S2CellIdTest.java
@@ -193,8 +193,9 @@ public strictfp class S2CellIdTest extends GeometryTestCase {
 
       // Check that the ToPointRaw() returns the center of each cell
       // in (s,t) coordinates.
-      R2Vector uv = new R2Vector();
-      S2Projections.xyzToFaceUV(id.toPointRaw(), uv);
+      S2Point p = id.toPointRaw();
+      int face = S2Projections.xyzToFace(p);
+      R2Vector uv = S2Projections.validFaceXyzToUv(face, p);
       assertDoubleNear(
           Math.IEEEremainder(S2Projections.uvToST(uv.x), 1.0 / (1 << MAX_WALK_LEVEL)), 0);
       assertDoubleNear(