From 94db475afed4129486aa0a2d2d02e159709d50f9 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Fri, 7 Mar 2014 08:58:48 +0100 Subject: [PATCH] DATACMNS-437 - Polishing of newly introduced geo value objects. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Polished JavaDoc. Simplified equals(…) and hashCode() methods where possible. Added additional null checks where appropriate. Removed obsolete generics in Polygon and switched to an immutable collection for the Points making up the Polygon. Related pull request: #68. --- .../org/springframework/data/geo/Box.java | 27 +++++++- .../org/springframework/data/geo/Circle.java | 11 ++-- .../springframework/data/geo/Distance.java | 32 +++++++--- .../org/springframework/data/geo/GeoPage.java | 4 ++ .../springframework/data/geo/GeoResult.java | 4 +- .../springframework/data/geo/GeoResults.java | 34 ++++++---- .../org/springframework/data/geo/Metrics.java | 2 + .../org/springframework/data/geo/Point.java | 45 +++++++++++--- .../org/springframework/data/geo/Polygon.java | 62 ++++++++++++------- .../org/springframework/data/geo/Shape.java | 1 - .../data/geo/package-info.java | 3 +- 11 files changed, 166 insertions(+), 59 deletions(-) diff --git a/src/main/java/org/springframework/data/geo/Box.java b/src/main/java/org/springframework/data/geo/Box.java index f3b67d34d..85aca0527 100644 --- a/src/main/java/org/springframework/data/geo/Box.java +++ b/src/main/java/org/springframework/data/geo/Box.java @@ -60,28 +60,52 @@ public class Box implements Shape { this.second = new Point(second[0], second[1]); } + /** + * Returns the first {@link Point} making up the {@link Box}. + * + * @return + */ public Point getFirst() { return first; } + /** + * Returns the second {@link Point} making up the {@link Box}. + * + * @return + */ public Point getSecond() { return second; } + /* + * (non-Javadoc) + * @see java.lang.Object#toString() + */ @Override public String toString() { return String.format("Box [%s, %s]", first, second); } + /* + * (non-Javadoc) + * @see java.lang.Object#hashCode() + */ @Override public int hashCode() { int result = 31; + result += 17 * first.hashCode(); result += 17 * second.hashCode(); + return result; } + /* + * (non-Javadoc) + * @see java.lang.Object#equals(java.lang.Object) + */ @Override public boolean equals(Object obj) { @@ -89,11 +113,12 @@ public class Box implements Shape { return true; } - if (obj == null || !(obj instanceof Box)) { + if (!(obj instanceof Box)) { return false; } Box that = (Box) obj; + return this.first.equals(that.first) && this.second.equals(that.second); } } diff --git a/src/main/java/org/springframework/data/geo/Circle.java b/src/main/java/org/springframework/data/geo/Circle.java index d4051528b..1d962571b 100644 --- a/src/main/java/org/springframework/data/geo/Circle.java +++ b/src/main/java/org/springframework/data/geo/Circle.java @@ -54,7 +54,6 @@ public class Circle implements Shape { * @param center must not be {@literal null}. * @param radius's value must be greater or equal to zero. */ - @PersistenceConstructor public Circle(Point center, double radius) { this(center, new Distance(radius)); } @@ -95,10 +94,11 @@ public class Circle implements Shape { */ @Override public String toString() { - return String.format("Circle [center=%s, radius=%f]", center, radius); + return String.format("Circle: [center=%s, radius=%f]", center, radius); } - /* (non-Javadoc) + /* + * (non-Javadoc) * @see java.lang.Object#equals(java.lang.Object) */ @Override @@ -108,7 +108,7 @@ public class Circle implements Shape { return true; } - if (obj == null || !(obj instanceof Circle)) { + if (!(obj instanceof Circle)) { return false; } @@ -123,9 +123,12 @@ public class Circle implements Shape { */ @Override public int hashCode() { + int result = 17; + result += 31 * center.hashCode(); result += 31 * radius.hashCode(); + return result; } } diff --git a/src/main/java/org/springframework/data/geo/Distance.java b/src/main/java/org/springframework/data/geo/Distance.java index e671c3b03..eb7086f93 100644 --- a/src/main/java/org/springframework/data/geo/Distance.java +++ b/src/main/java/org/springframework/data/geo/Distance.java @@ -15,7 +15,7 @@ */ package org.springframework.data.geo; -import org.springframework.util.ObjectUtils; +import org.springframework.util.Assert; /** * Value object to represent distances in a given metric. @@ -42,7 +42,7 @@ public class Distance { * Creates a new {@link Distance} with the given {@link Metric}. * * @param value - * @param metric + * @param metric can be {@literal null}. */ public Distance(double value, Metric metric) { this.value = value; @@ -50,6 +50,8 @@ public class Distance { } /** + * Returns the distance value in the current {@link Metric}. + * * @return the value */ public double getValue() { @@ -66,6 +68,8 @@ public class Distance { } /** + * Retruns the {@link Metric} of the {@link Distance}. + * * @return the metric */ public Metric getMetric() { @@ -76,24 +80,33 @@ public class Distance { * Adds the given distance to the current one. The resulting {@link Distance} will be in the same metric as the * current one. * - * @param other + * @param other must not be {@literal null}. * @return */ public Distance add(Distance other) { + + Assert.notNull(other, "Distance to add must not be null!"); + double newNormalizedValue = getNormalizedValue() + other.getNormalizedValue(); + return new Distance(newNormalizedValue * metric.getMultiplier(), metric); } /** * Adds the given {@link Distance} to the current one and forces the result to be in a given {@link Metric}. * - * @param other - * @param metric + * @param other must not be {@literal null}. + * @param metric must not be {@literal null}. * @return */ public Distance add(Distance other, Metric metric) { + + Assert.notNull(other, "Distance to must not be null!"); + Assert.notNull(metric, "Result metric must not be null!"); + double newLeft = getNormalizedValue() * metric.getMultiplier(); double newRight = other.getNormalizedValue() * metric.getMultiplier(); + return new Distance(newLeft + newRight, metric); } @@ -108,13 +121,13 @@ public class Distance { return true; } - if (obj == null || !getClass().isInstance(obj)) { + if (!(obj instanceof Distance)) { return false; } Distance that = (Distance) obj; - return this.value == that.value && ObjectUtils.nullSafeEquals(this.metric, that.metric); + return this.value == that.value && this.metric.equals(that.metric); } /* @@ -123,9 +136,12 @@ public class Distance { */ @Override public int hashCode() { + int result = 17; + result += 31 * Double.doubleToLongBits(value); - result += 31 * ObjectUtils.nullSafeHashCode(metric); + result += 31 * metric.hashCode(); + return result; } diff --git a/src/main/java/org/springframework/data/geo/GeoPage.java b/src/main/java/org/springframework/data/geo/GeoPage.java index f716307ee..e0603360e 100644 --- a/src/main/java/org/springframework/data/geo/GeoPage.java +++ b/src/main/java/org/springframework/data/geo/GeoPage.java @@ -38,7 +38,9 @@ public class GeoPage extends PageImpl> { * @param content must not be {@literal null}. */ public GeoPage(GeoResults results) { + super(results.getContent()); + this.averageDistance = results.getAverageDistance(); } @@ -50,7 +52,9 @@ public class GeoPage extends PageImpl> { * @param total */ public GeoPage(GeoResults results, Pageable pageable, long total) { + super(results.getContent(), pageable, total); + this.averageDistance = results.getAverageDistance(); } diff --git a/src/main/java/org/springframework/data/geo/GeoResult.java b/src/main/java/org/springframework/data/geo/GeoResult.java index e269a8695..b31f056be 100644 --- a/src/main/java/org/springframework/data/geo/GeoResult.java +++ b/src/main/java/org/springframework/data/geo/GeoResult.java @@ -73,7 +73,7 @@ public class GeoResult { return true; } - if (obj == null || !getClass().isInstance(obj)) { + if (!(obj instanceof GeoResult)) { return false; } @@ -90,8 +90,10 @@ public class GeoResult { public int hashCode() { int result = 17; + result += 31 * distance.hashCode(); result += 31 * content.hashCode(); + return result; } diff --git a/src/main/java/org/springframework/data/geo/GeoResults.java b/src/main/java/org/springframework/data/geo/GeoResults.java index 4c9d8d7a1..279b2fb46 100644 --- a/src/main/java/org/springframework/data/geo/GeoResults.java +++ b/src/main/java/org/springframework/data/geo/GeoResults.java @@ -45,6 +45,13 @@ public class GeoResults implements Iterable> { this(results, (Metric) null); } + /** + * Creates a new {@link GeoResults} instance manually calculating the average distance in the given {@link Metric} + * from the distance values of the given {@link GeoResult}s. + * + * @param results must not be {@literal null}. + * @param metric must not be {@literal null}. + */ public GeoResults(List> results, Metric metric) { this(results, calculateAverageDistance(results, metric)); } @@ -53,11 +60,13 @@ public class GeoResults implements Iterable> { * Creates a new {@link GeoResults} instance from the given {@link GeoResult}s and average distance. * * @param results must not be {@literal null}. - * @param averageDistance + * @param averageDistance can be {@literal null}. */ @PersistenceConstructor public GeoResults(List> results, Distance averageDistance) { + Assert.notNull(results); + this.results = results; this.averageDistance = averageDistance; } @@ -71,6 +80,15 @@ public class GeoResults implements Iterable> { return averageDistance; } + /** + * Returns the actual content of the {@link GeoResults}. + * + * @return + */ + public List> getContent() { + return Collections.unmodifiableList(results); + } + /* * (non-Javadoc) * @see java.lang.Iterable#iterator() @@ -80,15 +98,6 @@ public class GeoResults implements Iterable> { return (Iterator>) results.iterator(); } - /** - * Returns the actual - * - * @return - */ - public List> getContent() { - return Collections.unmodifiableList(results); - } - /* * (non-Javadoc) * @see java.lang.Object#equals(java.lang.Object) @@ -100,7 +109,7 @@ public class GeoResults implements Iterable> { return true; } - if (obj == null || !getClass().isInstance(obj)) { + if (!(obj instanceof GeoResults)) { return false; } @@ -115,9 +124,12 @@ public class GeoResults implements Iterable> { */ @Override public int hashCode() { + int result = 17; + result += 31 * results.hashCode(); result += 31 * averageDistance.hashCode(); + return result; } diff --git a/src/main/java/org/springframework/data/geo/Metrics.java b/src/main/java/org/springframework/data/geo/Metrics.java index 0fb4345d0..e30c0ec25 100644 --- a/src/main/java/org/springframework/data/geo/Metrics.java +++ b/src/main/java/org/springframework/data/geo/Metrics.java @@ -29,6 +29,8 @@ public enum Metrics implements Metric { private final double multiplier; /** + * Creates a new {@link Metrics} using the given muliplier. + * * @param multiplier the earth radius at equator. */ private Metrics(double multiplier) { diff --git a/src/main/java/org/springframework/data/geo/Point.java b/src/main/java/org/springframework/data/geo/Point.java index e5f21fb93..ff2cd529a 100644 --- a/src/main/java/org/springframework/data/geo/Point.java +++ b/src/main/java/org/springframework/data/geo/Point.java @@ -56,49 +56,76 @@ public class Point { this.y = point.y; } + /** + * Returns the x-coordinate of the {@link Point}. + * + * @return + */ public double getX() { return x; } + /** + * Returns the y-coordinate of the {@link Point}. + * + * @return + */ public double getY() { return y; } + /* + * (non-Javadoc) + * @see java.lang.Object#hashCode() + */ @Override public int hashCode() { - final int prime = 31; + int result = 1; - long temp; - temp = Double.doubleToLongBits(x); - result = prime * result + (int) (temp ^ (temp >>> 32)); + + long temp = Double.doubleToLongBits(x); + result = 31 * result + (int) (temp ^ temp >>> 32); + temp = Double.doubleToLongBits(y); - result = prime * result + (int) (temp ^ (temp >>> 32)); + result = 31 * result + (int) (temp ^ temp >>> 32); + return result; } + /* + * (non-Javadoc) + * @see java.lang.Object#equals(java.lang.Object) + */ @Override public boolean equals(Object obj) { + if (this == obj) { return true; } - if (obj == null) { - return false; - } + if (!(obj instanceof Point)) { return false; } + Point other = (Point) obj; + if (Double.doubleToLongBits(x) != Double.doubleToLongBits(other.x)) { return false; } + if (Double.doubleToLongBits(y) != Double.doubleToLongBits(other.y)) { return false; } + return true; } + /* + * (non-Javadoc) + * @see java.lang.Object#toString() + */ @Override public String toString() { - return String.format("Point [latitude=%f, longitude=%f]", x, y); + return String.format("Point [x=%f, y=%f]", x, y); } } diff --git a/src/main/java/org/springframework/data/geo/Polygon.java b/src/main/java/org/springframework/data/geo/Polygon.java index 7cc3b0094..51ff05505 100644 --- a/src/main/java/org/springframework/data/geo/Polygon.java +++ b/src/main/java/org/springframework/data/geo/Polygon.java @@ -17,10 +17,13 @@ package org.springframework.data.geo; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Iterator; import java.util.List; +import org.springframework.data.annotation.PersistenceConstructor; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; /** * Simple value object to represent a {@link Polygon}. @@ -36,49 +39,53 @@ public class Polygon implements Iterable, Shape { /** * Creates a new {@link Polygon} for the given Points. * - * @param x - * @param y - * @param z + * @param x must not be {@literal null}. + * @param y must not be {@literal null}. + * @param z must not be {@literal null}. * @param others */ - public

Polygon(P x, P y, P z, P... others) { + public Polygon(Point x, Point y, Point z, Point... others) { - Assert.notNull(x); - Assert.notNull(y); - Assert.notNull(z); + Assert.notNull(x, "X coordinate must not be null!"); + Assert.notNull(y, "Y coordinate must not be null!"); + Assert.notNull(z, "Z coordinate must not be null!"); Assert.notNull(others); - this.points = new ArrayList(3 + others.length); - this.points.addAll(Arrays.asList(x, y, z)); - this.points.addAll(Arrays.asList(others)); + List points = new ArrayList(3 + others.length); + points.addAll(Arrays.asList(x, y, z)); + points.addAll(Arrays.asList(others)); + + this.points = Collections.unmodifiableList(points); } /** * Creates a new {@link Polygon} for the given Points. * - * @param points + * @param points must not be {@literal null}. */ - public

Polygon(List

points) { + @PersistenceConstructor + public Polygon(List points) { Assert.notNull(points); - this.points = new ArrayList(points.size()); + List pointsToSet = new ArrayList(points.size()); for (Point point : points) { + Assert.notNull(point); - this.points.add(point); + pointsToSet.add(point); } + + this.points = Collections.unmodifiableList(pointsToSet); } + /** + * Returns all {@link Point}s the {@link Polygon} is made of. + * + * @return + */ public List getPoints() { - - List result = new ArrayList(); - - for (Point point : points) { - result.add(point); - } - - return result; + return this.points; } /* @@ -100,7 +107,7 @@ public class Polygon implements Iterable, Shape { return true; } - if (obj == null || !(obj instanceof Polygon)) { + if (!(obj instanceof Polygon)) { return false; } @@ -117,4 +124,13 @@ public class Polygon implements Iterable, Shape { public int hashCode() { return points.hashCode(); } + + /* + * (non-Javadoc) + * @see java.lang.Object#toString() + */ + @Override + public String toString() { + return String.format("Polygon: [%s]", StringUtils.collectionToCommaDelimitedString(points)); + } } diff --git a/src/main/java/org/springframework/data/geo/Shape.java b/src/main/java/org/springframework/data/geo/Shape.java index 560afdc96..8affdbd3e 100644 --- a/src/main/java/org/springframework/data/geo/Shape.java +++ b/src/main/java/org/springframework/data/geo/Shape.java @@ -15,7 +15,6 @@ */ package org.springframework.data.geo; - /** * Common interface for all shapes. Allows building external representations of them. * diff --git a/src/main/java/org/springframework/data/geo/package-info.java b/src/main/java/org/springframework/data/geo/package-info.java index d9ce1acdb..f7ca48c33 100644 --- a/src/main/java/org/springframework/data/geo/package-info.java +++ b/src/main/java/org/springframework/data/geo/package-info.java @@ -1,7 +1,8 @@ /** - * Support for reusable geospatial structures. + * Value types representing geo-spatial concepts. * * @author Thomas Darimont + * @author Oliver Gierke * @since 1.8 */ package org.springframework.data.geo; \ No newline at end of file