Make Geometry.reverse() consistent (#513)

* Make Geometry.reverse() consistent
* Add protected reverseInternal function to Geometry,
  implement in Point, LineString, LinearRing, Polygon and
  GeometryCollection
* Remove abstract from Geometry.reverse function,
  implementation calls reverseInternal and copies
  Envelope and SRID values
* To avoid breaking change make Geometry.reverse overrides
  return super.reverse()
* Add unit test
* Add @deprecated to Geometry.reverse() overrides
* Fix ExtractLineByLocation to not use deprecated
  Geometry.reverse() overrides
* Remove dead code
* Fix unit test LengthIndexedLineTest.testExtractLineReverseMulti

Note:
The reversal of items in MultiLineString has been removed.

relates to #305

Signed-off-by: Felix Obermaier <[email protected]>

modules/core/src/main/java/org/locationtech/jts/geom/GeometryCollection.java

@@ -13,7 +13,9 @@
  */
 package org.locationtech.jts.geom;
 
+import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.TreeSet;
 
 import org.locationtech.jts.util.Assert;
@@ -22,7 +24,7 @@
 /**
  * Models a collection of {@link Geometry}s of
  * arbitrary type and dimension.
- * 
+ *
  *
  *@version 1.7
  */
@@ -218,7 +220,7 @@ public void apply(GeometryComponentFilter filter) {
   public Object clone() {
     return copy();
   }
-  
+
   protected GeometryCollection copyInternal() {
     Geometry[] geometries = new Geometry[this.geometries.length];
     for (int i = 0; i < geometries.length; i++) {
@@ -266,26 +268,31 @@ protected int compareToSameClass(Object o, CoordinateSequenceComparator comp) {
     return 0;
 
   }
-  
+
   protected int getSortIndex() {
     return Geometry.SORTINDEX_GEOMETRYCOLLECTION;
   }
-  
+
   /**
    * Creates a {@link GeometryCollection} with
    * every component reversed.
    * The order of the components in the collection are not reversed.
    *
    * @return a {@link GeometryCollection} in the reverse order
+   * @deprecated
    */
-  public Geometry reverse()
+  public Geometry reverse() {
+    return super.reverse();
+  }
+
+  protected Geometry reverseInternal()
   {
-    int n = geometries.length;
-    Geometry[] revGeoms = new Geometry[n];
-    for (int i = 0; i < geometries.length; i++) {
-      revGeoms[i] = geometries[i].reverse();
+    int numGeometries = geometries.length;
+    Collection<Geometry> reversed = new ArrayList<>(numGeometries);
+    for (int i = 0; i < numGeometries; i++) {
+      reversed.add(geometries[i].reverse());
     }
-    return getFactory().createGeometryCollection(revGeoms);
+    return getFactory().buildGeometry(reversed);
   }
 }
 

modules/core/src/main/java/org/locationtech/jts/geom/LineString.java

@@ -18,21 +18,21 @@
  *  Models an OGC-style <code>LineString</code>.
  *  A LineString consists of a sequence of two or more vertices,
  *  along with all points along the linearly-interpolated curves
- *  (line segments) between each 
+ *  (line segments) between each
  *  pair of consecutive vertices.
  *  Consecutive vertices may be equal.
- *  The line segments in the line may intersect each other (in other words, 
+ *  The line segments in the line may intersect each other (in other words,
  *  the linestring may "curl back" in itself and self-intersect.
- *  Linestrings with exactly two identical points are invalid. 
- *  <p> 
- * A linestring must have either 0 or 2 or more points.  
- * If these conditions are not met, the constructors throw 
+ *  Linestrings with exactly two identical points are invalid.
+ *  <p>
+ * A linestring must have either 0 or 2 or more points.
+ * If these conditions are not met, the constructors throw
  * an {@link IllegalArgumentException}
  *
  *@version 1.7
  */
-public class LineString 
-	extends Geometry 
+public class LineString
+	extends Geometry
 	implements Lineal
 {
   private static final long serialVersionUID = 3110669828065365560L;
@@ -62,9 +62,9 @@ public LineString(Coordinate points[], PrecisionModel precisionModel, int SRID)
 
   /**
    * Constructs a <code>LineString</code> with the given points.
-   *  
+   *
    *@param  points the points of the linestring, or <code>null</code>
-   *      to create the empty geometry. 
+   *      to create the empty geometry.
    * @throws IllegalArgumentException if too few points are provided
    */
   public LineString(CoordinateSequence points, GeometryFactory factory) {
@@ -78,7 +78,7 @@ private void init(CoordinateSequence points)
       points = getFactory().getCoordinateSequenceFactory().create(new Coordinate[]{});
     }
     if (points.size() == 1) {
-      throw new IllegalArgumentException("Invalid number of points in LineString (found " 
+      throw new IllegalArgumentException("Invalid number of points in LineString (found "
       		+ points.size() + " - must be 0 or >= 2)");
     }
     this.points = points;
@@ -179,13 +179,17 @@ public Geometry getBoundary() {
    * order of this objects
    *
    * @return a {@link LineString} with coordinates in the reverse order
+   * @deprecated
    */
-  public Geometry reverse()
+  public Geometry reverse() {
+    return super.reverse();
+  }
+
+  protected Geometry reverseInternal()
   {
     CoordinateSequence seq = points.copy();
     CoordinateSequences.reverse(seq);
-    LineString revLine = getFactory().createLineString(seq);
-    return revLine;
+    return getFactory().createLineString(seq);
   }
 
   /**
@@ -233,7 +237,7 @@ public void apply(CoordinateFilter filter) {
       }
   }
 
-  public void apply(CoordinateSequenceFilter filter) 
+  public void apply(CoordinateSequenceFilter filter)
   {
     if (points.size() == 0)
       return;
@@ -264,7 +268,7 @@ public void apply(GeometryComponentFilter filter) {
   public Object clone() {
     return copy();
   }
-  
+
   protected LineString copyInternal() {
     return new LineString(points.copy(), factory);
   }
@@ -322,7 +326,7 @@ protected int compareToSameClass(Object o, CoordinateSequenceComparator comp)
     LineString line = (LineString) o;
     return comp.compare(this.points, line.points);
   }
-  
+
   protected int getSortIndex() {
     return Geometry.SORTINDEX_LINESTRING;
   }

modules/core/src/main/java/org/locationtech/jts/geom/LinearRing.java

@@ -21,9 +21,9 @@
  * and the interior of the ring must not self-intersect.
  * Either orientation of the ring is allowed.
  * <p>
- * A ring must have either 0 or 4 or more points.  
+ * A ring must have either 0 or 4 or more points.
  * The first and last points must be equal (in 2D).
- * If these conditions are not met, the constructors throw 
+ * If these conditions are not met, the constructors throw
  * an {@link IllegalArgumentException}
  *
  * @version 1.7
@@ -35,7 +35,7 @@ public class LinearRing extends LineString
    * Empty rings with 0 vertices are also valid.
    */
   public static final int MINIMUM_VALID_SIZE = 4;
-  
+
   private static final long serialVersionUID = -4261142084085851829L;
 
   /**
@@ -50,7 +50,7 @@ public class LinearRing extends LineString
    *@param  SRID            the ID of the Spatial Reference System used by this
    *      <code>LinearRing</code>
    * @throws IllegalArgumentException if the ring is not closed, or has too few points
-   * 
+   *
    * @deprecated Use GeometryFactory instead
    */
   public LinearRing(Coordinate points[], PrecisionModel precisionModel,
@@ -76,7 +76,7 @@ private LinearRing(Coordinate points[], GeometryFactory factory) {
    *
    *@param  points  a sequence points forming a closed and simple linestring, or
    *      <code>null</code> to create the empty geometry.
-   *      
+   *
    * @throws IllegalArgumentException if the ring is not closed, or has too few points
    *
    */
@@ -90,7 +90,7 @@ private void validateConstruction() {
       throw new IllegalArgumentException("Points of LinearRing do not form a closed linestring");
     }
     if (getCoordinateSequence().size() >= 1 && getCoordinateSequence().size() < MINIMUM_VALID_SIZE) {
-      throw new IllegalArgumentException("Invalid number of points in LinearRing (found " 
+      throw new IllegalArgumentException("Invalid number of points in LinearRing (found "
       		+ getCoordinateSequence().size() + " - must be 0 or >= 4)");
     }
   }
@@ -108,7 +108,7 @@ public int getBoundaryDimension() {
   /**
    * Tests whether this ring is closed.
    * Empty rings are closed by definition.
-   * 
+   *
    * @return true if this ring is closed
    */
   public boolean isClosed() {
@@ -123,20 +123,24 @@ public boolean isClosed() {
   public String getGeometryType() {
     return "LinearRing";
   }
-  
+
   protected int getSortIndex() {
     return Geometry.SORTINDEX_LINEARRING;
   }
-  
+
   protected LinearRing copyInternal() {
     return new LinearRing(points.copy(), factory);
   }
 
+  /** @deprecated */
   public Geometry reverse()
   {
+    return super.reverse();
+  }
+
+  public Geometry reverseInternal() {
     CoordinateSequence seq = points.copy();
     CoordinateSequences.reverse(seq);
-    LinearRing rev = getFactory().createLinearRing(seq);
-    return rev;
+    return getFactory().createLinearRing(seq);
   }
 }

modules/core/src/main/java/org/locationtech/jts/geom/MultiLineString.java

@@ -20,7 +20,7 @@
  *
  *@version 1.7
  */
-public class MultiLineString 
+public class MultiLineString
 	extends GeometryCollection
 	implements Lineal
 	{
@@ -102,17 +102,12 @@ public Geometry getBoundary()
    * are reversed.
    *
    * @return a {@link MultiLineString} in the reverse order
+   * @deprecated
    */
-  public Geometry reverse()
-  {
-    int nLines = geometries.length;
-    LineString[] revLines = new LineString[nLines];
-    for (int i = 0; i < geometries.length; i++) {
-      revLines[nLines - 1 - i] = (LineString)geometries[i].reverse();
-    }
-    return getFactory().createMultiLineString(revLines);
+  public Geometry reverse() {
+    return super.reverse();
   }
-  
+
   protected MultiLineString copyInternal() {
     LineString[] lineStrings = new LineString[this.geometries.length];
     for (int i = 0; i < lineStrings.length; i++) {

modules/core/src/main/java/org/locationtech/jts/geom/MultiPolygon.java

@@ -18,17 +18,17 @@
 /**
  * Models a collection of {@link Polygon}s.
  * <p>
- * As per the OGC SFS specification, 
- * the Polygons in a MultiPolygon may not overlap, 
+ * As per the OGC SFS specification,
+ * the Polygons in a MultiPolygon may not overlap,
  * and may only touch at single points.
  * This allows the topological point-set semantics
  * to be well-defined.
- *  
+ *
  *
  *@version 1.7
  */
-public class MultiPolygon 
-	extends GeometryCollection 
+public class MultiPolygon
+	extends GeometryCollection
 	implements Polygonal
 {
   private static final long serialVersionUID = -551033529766975875L;
@@ -83,7 +83,7 @@ public boolean isSimple() {
     return true;
   }
 */
-  
+
   /**
    * Computes the boundary of this geometry
    *
@@ -112,24 +112,19 @@ public boolean equalsExact(Geometry other, double tolerance) {
     }
     return super.equalsExact(other, tolerance);
   }
-  
+
   /**
    * Creates a {@link MultiPolygon} with
    * every component reversed.
    * The order of the components in the collection are not reversed.
    *
    * @return a MultiPolygon in the reverse order
+   * @deprecated
    */
-  public Geometry reverse()
-  {
-    int n = geometries.length;
-    Polygon[] revGeoms = new Polygon[n];
-    for (int i = 0; i < geometries.length; i++) {
-      revGeoms[i] = (Polygon) geometries[i].reverse();
-    }
-    return getFactory().createMultiPolygon(revGeoms);
+  public Geometry reverse() {
+    return super.reverse();
   }
-  
+
   protected MultiPolygon copyInternal() {
     Polygon[] polygons = new Polygon[this.geometries.length];
     for (int i = 0; i < polygons.length; i++) {