A tidyup CL for the geometry package. Each change in the CL is isolated to the
file it is in, so each file can be reviewed independantly.
Basically this tackles some very low hanging fruit with respect to immutability
and some minor style issues.
-------------
Created by MOE: http://code.google.com/p/moe-java
MOE_MIGRATED_REVID=24987273
diff --git a/src/com/google/common/geometry/S1Angle.java b/src/com/google/common/geometry/S1Angle.java
index 152052f..303de21 100644
--- a/src/com/google/common/geometry/S1Angle.java
+++ b/src/com/google/common/geometry/S1Angle.java
@@ -16,9 +16,9 @@
package com.google.common.geometry;
-public strictfp class S1Angle implements Comparable<S1Angle> {
+public final strictfp class S1Angle implements Comparable<S1Angle> {
- private double radians;
+ private final double radians;
public double radians() {
return radians;
diff --git a/src/com/google/common/geometry/S2Cap.java b/src/com/google/common/geometry/S2Cap.java
index 11fd0b9..b7b65b8 100644
--- a/src/com/google/common/geometry/S2Cap.java
+++ b/src/com/google/common/geometry/S2Cap.java
@@ -30,17 +30,17 @@
* h = 1 - cos(theta) = 2 sin^2(theta/2) d^2 = 2 h = a^2 + h^2
*
*/
-public strictfp class S2Cap implements S2Region {
+public final strictfp class S2Cap implements S2Region {
/**
* Multiply a positive number by this constant to ensure that the result of a
* floating point operation is at least as large as the true
* infinite-precision result.
*/
- static final double ROUND_UP = 1.0 + 1.0 / (1L << 52);
+ private static final double ROUND_UP = 1.0 + 1.0 / (1L << 52);
private final S2Point axis;
- private double height;
+ private final double height;
// Caps may be constructed from either an axis and a height, or an axis and
// an angle. To avoid ambiguity, there are no public constructors
diff --git a/src/com/google/common/geometry/S2Loop.java b/src/com/google/common/geometry/S2Loop.java
index 3ed2045..26b3740 100644
--- a/src/com/google/common/geometry/S2Loop.java
+++ b/src/com/google/common/geometry/S2Loop.java
@@ -48,7 +48,7 @@
*
*/
-public strictfp class S2Loop implements S2Region, Comparable<S2Loop> {
+public final strictfp class S2Loop implements S2Region, Comparable<S2Loop> {
private static final Logger log = Logger.getLogger(S2Loop.class.getCanonicalName());
/**
@@ -81,9 +81,6 @@
private boolean originInside;
private int depth;
- // TODO(kirilll): Get rid of debug mode. Turn it into tests.
- public static boolean debugMode = false;
-
/**
* Initialize a loop connecting the given vertices. The last vertex is
* implicitly connected to the first. All points should be unit length. Loops
diff --git a/src/com/google/common/geometry/S2Polygon.java b/src/com/google/common/geometry/S2Polygon.java
index 0ae8492..e3b93f7 100644
--- a/src/com/google/common/geometry/S2Polygon.java
+++ b/src/com/google/common/geometry/S2Polygon.java
@@ -57,7 +57,7 @@
* loop.
*
*/
-public strictfp class S2Polygon implements S2Region, Comparable<S2Polygon> {
+public final strictfp class S2Polygon implements S2Region, Comparable<S2Polygon> {
private static final Logger log = Logger.getLogger(S2Polygon.class.getCanonicalName());
private List<S2Loop> loops;
@@ -66,10 +66,6 @@
private boolean hasHoles;
private int numVertices;
- // TODO(kirilll): Get rid of debug mode. Turn it into tests. Should the debug
- // mode be set to false by default, anyways?
- public static boolean DEBUG = true;
-
/**
* Creates an empty polygon that should be initialized by calling Init().
*/
@@ -147,9 +143,7 @@
* hierarchy. (See also getParent and getLastDescendant.)
*/
public void init(List<S2Loop> loops) {
- if (DEBUG) {
- // assert (isValid(loops));
- }
+ // assert isValid(loops);
// assert (this.loops.isEmpty());
Map<S2Loop, List<S2Loop>> loopMap = Maps.newHashMap();
@@ -178,17 +172,8 @@
// Starting at null == starting at the root
initLoop(null, -1, loopMap);
- if (DEBUG) {
- // Check that the LoopMap is correct (this is fairly cheap).
- for (int i = 0; i < numLoops(); ++i) {
- for (int j = 0; j < numLoops(); ++j) {
- if (i == j) {
- continue;
- }
- // assert (containsChild(loop(i), loop(j), loopMap) == loop(i).containsNested(loop(j)));
- }
- }
- }
+ // TODO(dbeaumont): Add tests or preconditions for these asserts (here and elesewhere).
+ // forall i != j : containsChild(loop(i), loop(j), loopMap) == loop(i).containsNested(loop(j)));
// Compute the bounding rectangle of the entire polygon.
hasHoles = false;
diff --git a/src/com/google/common/geometry/S2Polyline.java b/src/com/google/common/geometry/S2Polyline.java
index 8b140f2..ebc8280 100644
--- a/src/com/google/common/geometry/S2Polyline.java
+++ b/src/com/google/common/geometry/S2Polyline.java
@@ -16,7 +16,6 @@
package com.google.common.geometry;
-import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Objects;
import com.google.common.base.Preconditions;
@@ -29,40 +28,32 @@
* straight edges (geodesics). Edges of length 0 and 180 degrees are not
* allowed, i.e. adjacent vertices should not be identical or antipodal.
*
+ * <p>Note: Polylines do not have a Contains(S2Point) method, because
+ * "containment" is not numerically well-defined except at the polyline
+ * vertices.
+ *
*/
-public strictfp class S2Polyline implements S2Region {
+public final strictfp class S2Polyline implements S2Region {
private static final Logger log = Logger.getLogger(S2Polyline.class.getCanonicalName());
- private int numVertices;
-
- private S2Point[] vertices;
-
- // TODO(kirilll): Get rid of debug mode. Turn it into tests.
- @VisibleForTesting
- static boolean debugMode = false;
+ private final int numVertices;
+ private final S2Point[] vertices;
/**
* Create a polyline that connects the given vertices. Empty polylines are
* allowed. Adjacent vertices should not be identical or antipodal. All
* vertices should be unit length.
- *
- * @param vertices
*/
public S2Polyline(List<S2Point> vertices) {
+ // assert isValid(vertices);
this.numVertices = vertices.size();
- this.vertices = new S2Point[numVertices];
-
- if (debugMode) {
- // assert (isValid(vertices));
- }
-
- if (numVertices > 0) {
- vertices.toArray(this.vertices);
- }
+ this.vertices = vertices.toArray(new S2Point[numVertices]);
}
/**
* Copy constructor.
+ *
+ * TODO(dbeaumont): Now that S2Polyline is immutable, remove this.
*/
public S2Polyline(S2Polyline src) {
this.numVertices = src.numVertices();
@@ -239,7 +230,6 @@
minIndex = i;
}
}
-
return minIndex;
}
@@ -264,7 +254,6 @@
}
S2Polyline thatPolygon = (S2Polyline) that;
-
if (numVertices != thatPolygon.numVertices) {
return false;
}
@@ -274,7 +263,6 @@
return false;
}
}
-
return true;
}
@@ -282,8 +270,4 @@
public int hashCode() {
return Objects.hashCode(numVertices, Arrays.deepHashCode(vertices));
}
-
-
- // Polylines do not have a Contains(S2Point) method, because "containment"
- // is not numerically well-defined except at the polyline vertices.
}
diff --git a/src/com/google/common/geometry/S2RegionCoverer.java b/src/com/google/common/geometry/S2RegionCoverer.java
index 14e62b1..a7e5147 100644
--- a/src/com/google/common/geometry/S2RegionCoverer.java
+++ b/src/com/google/common/geometry/S2RegionCoverer.java
@@ -53,7 +53,7 @@
* methods will conflict and produce unpredictable results.
*
*/
-public strictfp class S2RegionCoverer {
+public final strictfp class S2RegionCoverer {
/**
* By default, the covering uses at most 8 cells at any level. This gives a
diff --git a/tests/com/google/common/geometry/S2LatLngTest.java b/tests/com/google/common/geometry/S2LatLngTest.java
index 6a2ee24..da21e16 100644
--- a/tests/com/google/common/geometry/S2LatLngTest.java
+++ b/tests/com/google/common/geometry/S2LatLngTest.java
@@ -86,5 +86,4 @@
S2LatLng.fromDegrees(47, -127).getDistance(S2LatLng.fromDegrees(-47, 53)).degrees(), 180,
2e-6);
}
-
}
diff --git a/tests/com/google/common/geometry/S2LoopTest.java b/tests/com/google/common/geometry/S2LoopTest.java
index 648de70..fb05b05 100644
--- a/tests/com/google/common/geometry/S2LoopTest.java
+++ b/tests/com/google/common/geometry/S2LoopTest.java
@@ -88,8 +88,6 @@
@Override
public void setUp() {
super.setUp();
- S2Loop.debugMode = true;
-
southHemi = new S2Loop(northHemi);
southHemi.invert();
diff --git a/tests/com/google/common/geometry/S2PolygonBuilderTest.java b/tests/com/google/common/geometry/S2PolygonBuilderTest.java
index 9837062..ed47b93 100644
--- a/tests/com/google/common/geometry/S2PolygonBuilderTest.java
+++ b/tests/com/google/common/geometry/S2PolygonBuilderTest.java
@@ -238,14 +238,6 @@
+ "-8.5:-8.5, -8.5:0.5, -8.5:8.5, 0.5:8.5"},
0)};
- @Override
- public void setUp() {
- super.setUp();
- S2Loop.debugMode = true;
- S2Polygon.DEBUG = true;
- S2Polyline.debugMode = true;
- }
-
private void getVertices(String str,
S2Point x,
S2Point y,
diff --git a/tests/com/google/common/geometry/S2PolygonTest.java b/tests/com/google/common/geometry/S2PolygonTest.java
index 89a94ff..82c17c3 100644
--- a/tests/com/google/common/geometry/S2PolygonTest.java
+++ b/tests/com/google/common/geometry/S2PolygonTest.java
@@ -73,13 +73,6 @@
private static final String TRIANGLE = "15:0, 17:0, 16:2;";
private static final String TRIANGLE_ROT = "17:0, 16:2, 15:0;";
- @Override
- public void setUp() {
- super.setUp();
- S2Loop.debugMode = true;
- S2Polygon.DEBUG = true;
- }
-
private void assertContains(String aStr, String bStr) {
S2Polygon a = makePolygon(aStr);
S2Polygon b = makePolygon(bStr);
diff --git a/tests/com/google/common/geometry/S2PolylineTest.java b/tests/com/google/common/geometry/S2PolylineTest.java
index 1827d61..abce80e 100644
--- a/tests/com/google/common/geometry/S2PolylineTest.java
+++ b/tests/com/google/common/geometry/S2PolylineTest.java
@@ -30,7 +30,6 @@
@Override
public void setUp() {
super.setUp();
- S2Polyline.debugMode = true;
}
public void testBasic() {
