Fixed some code smells. (#180)

* Location: changed Double.valueOf to Double.parseDouble * MultiPolygon: removed some unnecessary curly braces/return statement in lambda * MultiPolygon: replaced lambda with method reference in bounds function * MultiPolygon: removed redundant continue statement in fullyGeometricallyEncloses(Location) * Polygon: merged nested if statements in isApproximatelyNSided * Polygon: merged if statement with enclosing one in overlapsInternal() * AtlasFindEntitiesByIdSubCommand: changed printf to println * AtlasResourceLoaderErrorSubCommand: removed extra curly braces * Applied spotless * Added explicit exception to Location forString * Addressed PR comment change in AtlasResourceLoaderErrorSubCommand
osmlab · Jul 25, 2018 · 3740c67 · 3740c67
1 parent b0e83d7
commit 3740c67
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 34 deletions.
diff --git a/src/main/java/org/openstreetmap/atlas/geography/Location.java b/src/main/java/org/openstreetmap/atlas/geography/Location.java
@@ -55,16 +55,18 @@ public class Location implements Located, Iterable<Location>, Serializable
      *            The {@link Location} as a {@link String} in "latitude(degrees),longitude(degrees)"
      *            format
      * @return The corresponding {@link Location}
+     * @throws NumberFormatException
+     *             if the latitude or longitude in the string is not a valid Double
      */
-    public static Location forString(final String locationString)
+    public static Location forString(final String locationString) throws NumberFormatException
     {
         final StringList split = StringList.split(locationString, ",");
         if (split.size() != 2)
         {
             throw new CoreException("Invalid Location String: {}", locationString);
         }
-        final double latitude = Double.valueOf(split.get(0));
-        final double longitude = Double.valueOf(split.get(1));
+        final double latitude = Double.parseDouble(split.get(0));
+        final double longitude = Double.parseDouble(split.get(1));
         return new Location(Latitude.degrees(latitude), Longitude.degrees(longitude));
     }
 
@@ -73,16 +75,19 @@ public static Location forString(final String locationString)
      *            The {@link Location} as a {@link String} in "longitude(degrees),latitude(degrees)"
      *            format
      * @return The corresponding {@link Location}
+     * @throws NumberFormatException
+     *             if the latitude or longitude in the string is not a valid Double
      */
     public static Location forStringLongitudeLatitude(final String locationString)
+            throws NumberFormatException
     {
         final StringList split = StringList.split(locationString, ",");
         if (split.size() != 2)
         {
             throw new CoreException("Invalid Location String: {}", locationString);
         }
-        final double latitude = Double.valueOf(split.get(1));
-        final double longitude = Double.valueOf(split.get(0));
+        final double latitude = Double.parseDouble(split.get(1));
+        final double longitude = Double.parseDouble(split.get(0));
         return new Location(Latitude.degrees(latitude), Longitude.degrees(longitude));
     }
 

diff --git a/src/main/java/org/openstreetmap/atlas/geography/MultiPolygon.java b/src/main/java/org/openstreetmap/atlas/geography/MultiPolygon.java
@@ -99,11 +99,10 @@ public GeoJsonObject asGeoJson()
     public GeoJsonObject asGeoJsonFeatureCollection()
     {
         final GeoJsonBuilder builder = new GeoJsonBuilder();
-        return builder.createFeatureCollection(Iterables.translate(outers(), outerPolygon ->
-        {
-            return builder.createOneOuterMultiPolygon(new MultiIterable<>(
-                    Collections.singleton(outerPolygon), this.outerToInners.get(outerPolygon)));
-        }));
+        return builder.createFeatureCollection(Iterables.translate(outers(),
+                outerPolygon -> builder.createOneOuterMultiPolygon(
+                        new MultiIterable<>(Collections.singleton(outerPolygon),
+                                this.outerToInners.get(outerPolygon)))));
     }
 
     public Iterable<LocationIterableProperties> asLocationIterableProperties()
@@ -142,7 +141,7 @@ public Rectangle bounds()
         if (this.bounds == null && !this.isEmpty())
         {
             final Set<Location> locations = new HashSet<>();
-            forEach(polygon -> polygon.forEach(location -> locations.add(location)));
+            forEach(polygon -> polygon.forEach(locations::add));
             this.bounds = Rectangle.forLocations(locations);
         }
         return this.bounds;
@@ -241,10 +240,6 @@ public boolean fullyGeometricallyEncloses(final Location location)
                 {
                     return true;
                 }
-                else
-                {
-                    continue;
-                }
             }
         }
         return false;

diff --git a/src/main/java/org/openstreetmap/atlas/geography/Polygon.java b/src/main/java/org/openstreetmap/atlas/geography/Polygon.java
@@ -346,15 +346,14 @@ public boolean isApproximatelyNSided(final int expectedNumberOfSides, final Angl
         while (segmentIndex < segmentSize && headingChangeCount <= expectedHeadingChangeCount)
         {
             final Optional<Heading> nextHeading = segments.get(segmentIndex++).heading();
-            if (nextHeading.isPresent())
+
+            // If heading difference is greater than threshold, then increment heading change
+            // counter and update previous heading, which is used as reference
+            if (nextHeading.isPresent()
+                    && previousHeading.get().difference(nextHeading.get()).isGreaterThan(threshold))
             {
-                // If heading difference is greater than threshold, then increment heading change
-                // counter and update previous heading, which is used as reference
-                if (previousHeading.get().difference(nextHeading.get()).isGreaterThan(threshold))
-                {
-                    headingChangeCount++;
-                    previousHeading = nextHeading;
-                }
+                headingChangeCount++;
+                previousHeading = nextHeading;
             }
         }
 
@@ -684,12 +683,10 @@ private boolean overlapsInternal(final PolyLine polyline, final boolean runRever
                 return true;
             }
         }
-        if (runReverseCheck && polyline instanceof Polygon)
+        if (runReverseCheck && polyline instanceof Polygon
+                && ((Polygon) polyline).overlapsInternal(this, false))
         {
-            if (((Polygon) polyline).overlapsInternal(this, false))
-            {
-                return true;
-            }
+            return true;
         }
         return this.intersects(polyline);
     }

diff --git a/...java/org/openstreetmap/atlas/geography/atlas/command/AtlasFindEntitiesByIdSubCommand.java b/...java/org/openstreetmap/atlas/geography/atlas/command/AtlasFindEntitiesByIdSubCommand.java
@@ -43,8 +43,8 @@ public SwitchList switches()
     public void usage(final PrintStream writer)
     {
         super.usage(writer);
-        writer.printf(
-                "-osmid=OSM identifier : comma-separated numeric osm identifiers of the items we're trying to locate\n");
+        writer.println(
+                "-osmid=OSM identifier : comma-separated numeric osm identifiers of the items we're trying to locate");
     }
 
     @Override

diff --git a/...a/org/openstreetmap/atlas/geography/atlas/command/AtlasResourceLoaderErrorSubCommand.java b/...a/org/openstreetmap/atlas/geography/atlas/command/AtlasResourceLoaderErrorSubCommand.java
@@ -36,10 +36,7 @@ public int execute(final CommandMap map)
         {
             oops.printStackTrace();
             ExceptionSearch.find(StreamCorruptedException.class).within(oops)
-                    .ifPresent(poorlyFormattedFile ->
-                    {
-                        poorlyFormattedFile.printStackTrace();
-                    });
+                    .ifPresent(StreamCorruptedException::printStackTrace);
         }
         return 0;
     }