[GitHub] [sedona] Kimahriman commented on a diff in pull request #742: [SEDONA-223] Add ST_Split

[GitBox <gi...@apache.org>]

Kimahriman commented on code in PR #742:
URL: https://github.com/apache/sedona/pull/742#discussion_r1062394602


##########
common/src/main/java/org/apache/sedona/common/utils/GeometrySplitter.java:
##########
@@ -0,0 +1,331 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sedona.common.utils;
+
+import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Deque;
+import java.util.Iterator;
+import java.util.List;
+
+import org.apache.log4j.Logger;
+import org.locationtech.jts.geom.Coordinate;
+import org.locationtech.jts.geom.Geometry;
+import org.locationtech.jts.geom.GeometryCollection;
+import org.locationtech.jts.geom.GeometryFactory;
+import org.locationtech.jts.geom.LineString;
+import org.locationtech.jts.geom.MultiLineString;
+import org.locationtech.jts.geom.MultiPoint;
+import org.locationtech.jts.geom.MultiPolygon;
+import org.locationtech.jts.geom.Point;
+import org.locationtech.jts.geom.Polygon;
+import org.locationtech.jts.linearref.LinearGeometryBuilder;
+import org.locationtech.jts.operation.polygonize.Polygonizer;
+
+
+/**
+ * Class to split geometry by other geometry.
+ */
+public final class GeometrySplitter {
+    private final GeometryFactory geometryFactory;
+
+    public GeometrySplitter(GeometryFactory geometryFactory) {
+        this.geometryFactory = geometryFactory;
+    }
+
+    /**
+     * Split input geometry by the blade geometry.
+     * Input geometry can be lineal (LineString or MultiLineString)
+     * or polygonal (Polygon or MultiPolygon). A GeometryCollection
+     * can also be used as an input but it must be homogeneous.
+     * For lineal geometry refer to the
+     * {@link splitLines(Geometry, Geometry) splitLines} method for
+     * restrictions on the blade. Refer to
+     * {@link splitPolygons(Geometry, Geometry) splitPolygons} for
+     * restrictions on the blade for polygonal input geometry.
+     * <p>
+     * The result will be null if the input geometry and blade are either
+     * invalid in general or in relation to eachother. Otherwise, the result
+     * will always be a MultiLineString or MultiPolygon depending on the input,
+     * and even if the result is a single geometry.
+     *
+     * @param  input input geometry
+     * @param  blade geometry to use as a blade
+     * @return       multi-geometry resulting from the split or null if invalid
+     */
+    public GeometryCollection split(Geometry input, Geometry blade) {
+        GeometryCollection result = null;
+
+        if (GeomUtils.geometryIsLineal(input)) {
+            result = splitLines(input, blade);
+        } else if (GeomUtils.geometryIsPolygonal(input)) {
+            result = splitPolygons(input, blade);
+        }
+
+        return result;
+    }
+
+    /**
+     * Split linear input geometry by the blade geometry.
+     * Input geometry is assumed to be either a LineString,
+     * MultiLineString, or a homogeneous collection of lines in a
+     * GeometryCollection. The blade geometry can be any indivdual
+     * puntal, lineal, or polygonal geometry or homogeneous collection
+     * of those geometries. Blades that are polygonal will use their
+     * boundary for the split. Will always return a MultiLineString.
+     *
+     * @param  input input geometry to be split that must be lineal
+     * @param  blade blade geometry to use for split
+     *
+     * @return       input geometry split by blade
+     */
+    public MultiLineString splitLines(Geometry input, Geometry blade) {
+        MultiLineString result = null;
+
+        if (GeomUtils.geometryIsPolygonal(blade)) {
+            blade = blade.getBoundary();
+        }
+
+        if (GeomUtils.geometryIsPuntal(blade)) {
+            result = splitLinesByPoints(input, blade);
+        } else if (GeomUtils.geometryIsLineal(blade)) {
+            result = splitLinesByLines(input, blade);
+        }
+
+        return result;
+    }
+
+    /**
+     * Split polygonal input geometry by the blade geometry.
+     * Input geometry is assumed to be either a Polygon, MultiPolygon,
+     * or a GeometryCollection of only polygons. The blade geometry
+     * can be any individual lineal or polygonal geometry or homogeneous
+     * collection of those geometries. Blades that are polygonal
+     * will use their boundary for the split. Will always return a
+     * MultiPolygon.
+     *
+     * @param  input input polygonal geometry to split
+     * @param  blade geometry to split the input by
+     * @return       input geometry split by the blade
+     */
+    public MultiPolygon splitPolygons(Geometry input, Geometry blade) {
+        MultiPolygon result = null;
+
+        if (GeomUtils.geometryIsPolygonal(blade)) {
+            blade = blade.getBoundary();
+        }
+
+        if (GeomUtils.geometryIsLineal(blade)) {
+            result = splitPolygonsByLines(input, blade);
+        }
+
+        return result;
+    }
+
+    private MultiLineString splitLinesByPoints(Geometry lines, Geometry points) {
+        // coords must be ordered for the algorithm in splitLineStringAtCoordinates
+        // do it here so the sorting is only done once per split operation
+        // use a deque for easy forward and reverse iteration
+        Deque<Coordinate> pointCoords = getOrderedCoords(points);
+
+        LinearGeometryBuilder lineBuilder = new LinearGeometryBuilder(geometryFactory);
+        lineBuilder.setIgnoreInvalidLines(true);
+
+        for (int lineIndex = 0; lineIndex < lines.getNumGeometries(); lineIndex++) {
+            splitLineStringAtCoordinates((LineString)lines.getGeometryN(lineIndex), pointCoords, lineBuilder);
+        }
+
+        MultiLineString result = (MultiLineString)ensureMultiGeometryOfDimensionN(lineBuilder.getGeometry(), 1);
+
+        return result;
+    }
+
+    private MultiLineString splitLinesByLines(Geometry inputLines, Geometry blade) {
+        // compute the intersection of inputLines and blade
+        // and pass back to splitLines to handle as points
+        Geometry intersectionWithBlade = inputLines.intersection(blade);
+
+        if (intersectionWithBlade.isEmpty()) {
+            // blade and inputLines are disjoint so just return the input as a multilinestring
+            return (MultiLineString)ensureMultiGeometryOfDimensionN(inputLines, 1);
+        } else if (intersectionWithBlade.getDimension() != 0) {
+            Logger.getLogger(GeometrySplitter.class).error("Colinear sections detected between source and blade geometry. Returned null.");

Review Comment:
   3 things:
   Can you try changing the common dependency from
   ```
           <dependency>
               <groupId>org.apache.logging.log4j</groupId>
               <artifactId>log4j-1.2-api</artifactId>
           </dependency>
   ```
   to
   ```
               <dependency>
                   <groupId>org.slf4j</groupId>
                   <artifactId>slf4j-api</artifactId>
               </dependency>
   ```
   and try using the slf4j logger API instead? I think doing that in the common module would be a good place to try to stop using the log4j 1.x API directly.
   
   Create the Logger as a static variable at the top of the class, I think that's the recommended way.
   
   And kinda just my opinion, but if it doesn't break/stop the execution, I feel like it should be.a warning message and not an error message



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org