Skip to content

Commit 9d370a8

Browse files
hagbardMichael Bolin
authored andcommitted
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
1 parent 1bc1d9c commit 9d370a8

11 files changed

Lines changed: 22 additions & 75 deletions

File tree

src/com/google/common/geometry/S1Angle.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@
1616
package com.google.common.geometry;
1717

1818

19-
public strictfp class S1Angle implements Comparable<S1Angle> {
19+
public final strictfp class S1Angle implements Comparable<S1Angle> {
2020

21-
private double radians;
21+
private final double radians;
2222

2323
public double radians() {
2424
return radians;

src/com/google/common/geometry/S2Cap.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,17 +30,17 @@
3030
* h = 1 - cos(theta) = 2 sin^2(theta/2) d^2 = 2 h = a^2 + h^2
3131
*
3232
*/
33-
public strictfp class S2Cap implements S2Region {
33+
public final strictfp class S2Cap implements S2Region {
3434

3535
/**
3636
* Multiply a positive number by this constant to ensure that the result of a
3737
* floating point operation is at least as large as the true
3838
* infinite-precision result.
3939
*/
40-
static final double ROUND_UP = 1.0 + 1.0 / (1L << 52);
40+
private static final double ROUND_UP = 1.0 + 1.0 / (1L << 52);
4141

4242
private final S2Point axis;
43-
private double height;
43+
private final double height;
4444

4545
// Caps may be constructed from either an axis and a height, or an axis and
4646
// an angle. To avoid ambiguity, there are no public constructors

src/com/google/common/geometry/S2Loop.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
*
4949
*/
5050

51-
public strictfp class S2Loop implements S2Region, Comparable<S2Loop> {
51+
public final strictfp class S2Loop implements S2Region, Comparable<S2Loop> {
5252
private static final Logger log = Logger.getLogger(S2Loop.class.getCanonicalName());
5353

5454
/**
@@ -81,9 +81,6 @@ public strictfp class S2Loop implements S2Region, Comparable<S2Loop> {
8181
private boolean originInside;
8282
private int depth;
8383

84-
// TODO(kirilll): Get rid of debug mode. Turn it into tests.
85-
public static boolean debugMode = false;
86-
8784
/**
8885
* Initialize a loop connecting the given vertices. The last vertex is
8986
* implicitly connected to the first. All points should be unit length. Loops

src/com/google/common/geometry/S2Polygon.java

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@
5757
* loop.
5858
*
5959
*/
60-
public strictfp class S2Polygon implements S2Region, Comparable<S2Polygon> {
60+
public final strictfp class S2Polygon implements S2Region, Comparable<S2Polygon> {
6161
private static final Logger log = Logger.getLogger(S2Polygon.class.getCanonicalName());
6262

6363
private List<S2Loop> loops;
@@ -66,10 +66,6 @@ public strictfp class S2Polygon implements S2Region, Comparable<S2Polygon> {
6666
private boolean hasHoles;
6767
private int numVertices;
6868

69-
// TODO(kirilll): Get rid of debug mode. Turn it into tests. Should the debug
70-
// mode be set to false by default, anyways?
71-
public static boolean DEBUG = true;
72-
7369
/**
7470
* Creates an empty polygon that should be initialized by calling Init().
7571
*/
@@ -147,9 +143,7 @@ public int compareTo(S2Polygon other) {
147143
* hierarchy. (See also getParent and getLastDescendant.)
148144
*/
149145
public void init(List<S2Loop> loops) {
150-
if (DEBUG) {
151-
// assert (isValid(loops));
152-
}
146+
// assert isValid(loops);
153147
// assert (this.loops.isEmpty());
154148

155149
Map<S2Loop, List<S2Loop>> loopMap = Maps.newHashMap();
@@ -178,17 +172,8 @@ public void init(List<S2Loop> loops) {
178172
// Starting at null == starting at the root
179173
initLoop(null, -1, loopMap);
180174

181-
if (DEBUG) {
182-
// Check that the LoopMap is correct (this is fairly cheap).
183-
for (int i = 0; i < numLoops(); ++i) {
184-
for (int j = 0; j < numLoops(); ++j) {
185-
if (i == j) {
186-
continue;
187-
}
188-
// assert (containsChild(loop(i), loop(j), loopMap) == loop(i).containsNested(loop(j)));
189-
}
190-
}
191-
}
175+
// TODO(dbeaumont): Add tests or preconditions for these asserts (here and elesewhere).
176+
// forall i != j : containsChild(loop(i), loop(j), loopMap) == loop(i).containsNested(loop(j)));
192177

193178
// Compute the bounding rectangle of the entire polygon.
194179
hasHoles = false;

src/com/google/common/geometry/S2Polyline.java

Lines changed: 11 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
package com.google.common.geometry;
1818

19-
import com.google.common.annotations.VisibleForTesting;
2019
import com.google.common.base.Objects;
2120
import com.google.common.base.Preconditions;
2221

@@ -29,40 +28,32 @@
2928
* straight edges (geodesics). Edges of length 0 and 180 degrees are not
3029
* allowed, i.e. adjacent vertices should not be identical or antipodal.
3130
*
31+
* <p>Note: Polylines do not have a Contains(S2Point) method, because
32+
* "containment" is not numerically well-defined except at the polyline
33+
* vertices.
34+
*
3235
*/
33-
public strictfp class S2Polyline implements S2Region {
36+
public final strictfp class S2Polyline implements S2Region {
3437
private static final Logger log = Logger.getLogger(S2Polyline.class.getCanonicalName());
3538

36-
private int numVertices;
37-
38-
private S2Point[] vertices;
39-
40-
// TODO(kirilll): Get rid of debug mode. Turn it into tests.
41-
@VisibleForTesting
42-
static boolean debugMode = false;
39+
private final int numVertices;
40+
private final S2Point[] vertices;
4341

4442
/**
4543
* Create a polyline that connects the given vertices. Empty polylines are
4644
* allowed. Adjacent vertices should not be identical or antipodal. All
4745
* vertices should be unit length.
48-
*
49-
* @param vertices
5046
*/
5147
public S2Polyline(List<S2Point> vertices) {
48+
// assert isValid(vertices);
5249
this.numVertices = vertices.size();
53-
this.vertices = new S2Point[numVertices];
54-
55-
if (debugMode) {
56-
// assert (isValid(vertices));
57-
}
58-
59-
if (numVertices > 0) {
60-
vertices.toArray(this.vertices);
61-
}
50+
this.vertices = vertices.toArray(new S2Point[numVertices]);
6251
}
6352

6453
/**
6554
* Copy constructor.
55+
*
56+
* TODO(dbeaumont): Now that S2Polyline is immutable, remove this.
6657
*/
6758
public S2Polyline(S2Polyline src) {
6859
this.numVertices = src.numVertices();
@@ -239,7 +230,6 @@ public int getNearestEdgeIndex(S2Point point) {
239230
minIndex = i;
240231
}
241232
}
242-
243233
return minIndex;
244234
}
245235

@@ -264,7 +254,6 @@ public boolean equals(Object that) {
264254
}
265255

266256
S2Polyline thatPolygon = (S2Polyline) that;
267-
268257
if (numVertices != thatPolygon.numVertices) {
269258
return false;
270259
}
@@ -274,16 +263,11 @@ public boolean equals(Object that) {
274263
return false;
275264
}
276265
}
277-
278266
return true;
279267
}
280268

281269
@Override
282270
public int hashCode() {
283271
return Objects.hashCode(numVertices, Arrays.deepHashCode(vertices));
284272
}
285-
286-
287-
// Polylines do not have a Contains(S2Point) method, because "containment"
288-
// is not numerically well-defined except at the polyline vertices.
289273
}

src/com/google/common/geometry/S2RegionCoverer.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@
5353
* methods will conflict and produce unpredictable results.
5454
*
5555
*/
56-
public strictfp class S2RegionCoverer {
56+
public final strictfp class S2RegionCoverer {
5757

5858
/**
5959
* By default, the covering uses at most 8 cells at any level. This gives a

tests/com/google/common/geometry/S2LatLngTest.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,5 +86,4 @@ public void testDistance() {
8686
S2LatLng.fromDegrees(47, -127).getDistance(S2LatLng.fromDegrees(-47, 53)).degrees(), 180,
8787
2e-6);
8888
}
89-
9089
}

tests/com/google/common/geometry/S2LoopTest.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,6 @@ public strictfp class S2LoopTest extends GeometryTestCase {
8888
@Override
8989
public void setUp() {
9090
super.setUp();
91-
S2Loop.debugMode = true;
92-
9391
southHemi = new S2Loop(northHemi);
9492
southHemi.invert();
9593

tests/com/google/common/geometry/S2PolygonBuilderTest.java

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -238,14 +238,6 @@ public TestCase(int undirectedEdges,
238238
+ "-8.5:-8.5, -8.5:0.5, -8.5:8.5, 0.5:8.5"},
239239
0)};
240240

241-
@Override
242-
public void setUp() {
243-
super.setUp();
244-
S2Loop.debugMode = true;
245-
S2Polygon.DEBUG = true;
246-
S2Polyline.debugMode = true;
247-
}
248-
249241
private void getVertices(String str,
250242
S2Point x,
251243
S2Point y,

tests/com/google/common/geometry/S2PolygonTest.java

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -73,13 +73,6 @@ public strictfp class S2PolygonTest extends GeometryTestCase {
7373
private static final String TRIANGLE = "15:0, 17:0, 16:2;";
7474
private static final String TRIANGLE_ROT = "17:0, 16:2, 15:0;";
7575

76-
@Override
77-
public void setUp() {
78-
super.setUp();
79-
S2Loop.debugMode = true;
80-
S2Polygon.DEBUG = true;
81-
}
82-
8376
private void assertContains(String aStr, String bStr) {
8477
S2Polygon a = makePolygon(aStr);
8578
S2Polygon b = makePolygon(bStr);

0 commit comments

Comments
 (0)