Changeset 19065 in josm

Timestamp:

2024-04-28T09:37:20+02:00 (7 months ago)

Author:

GerdP

Message:

see #21881 Add a check for loops in directional waterways
Fix code in WaySegment and IWaySegment

• change text in Exception to "Node pair is not a single segment of the way!" and move it to constant IWaySegment.NOT_A_SEGMENT
• fix error in WaySegment.forNodePair and IWaySegment.forNodePair
• add unit test for further invalid node pairs

Location:

trunk

Files:

• src/org/openstreetmap/josm/data/osm/IWaySegment.java (modified) (5 diffs)
• src/org/openstreetmap/josm/data/osm/WaySegment.java (modified) (3 diffs)
• test/unit/org/openstreetmap/josm/data/osm/WaySegmentTest.java (modified) (1 diff)

trunk/src/org/openstreetmap/josm/data/osm/IWaySegment.java

@@ -18 +18 @@
  */
 public class IWaySegment<N extends INode, W extends IWay<N>> implements Comparable<IWaySegment<N, W>> {
+    protected static final String NOT_A_SEGMENT = "Node pair is not a single segment of the way!";
 
     private final W way;
@@ -97 +98 @@
      * @param second second node
      * @return way segment
-     * @throws IllegalArgumentException if the node pair is not part of way
+     * @throws IllegalArgumentException if the node pair is not a single segment of the way
      */
     public static <N extends INode, W extends IWay<N>> IWaySegment<N, W> forNodePair(W way, N first, N second) {
@@ -103 +104 @@
         while (endIndex > 0) {
             final int indexOfFirst = way.getNodes().subList(0, endIndex).lastIndexOf(first);
+            if (indexOfFirst < 0)
+                break;
             if (second.equals(way.getNode(indexOfFirst + 1))) {
                 return new IWaySegment<>(way, indexOfFirst);
@@ -108 +111 @@
             endIndex--;
         }
-        throw new IllegalArgumentException("Node pair is not part of way!");
+        throw new IllegalArgumentException(NOT_A_SEGMENT);
     }
 
@@ -162 +165 @@
     @Override
     public int compareTo(IWaySegment o) {
+        if (o == null)
+            return -1;
         final W thisWay;
         final IWay<?> otherWay;
         try {
             thisWay = toWay();
-            otherWay = o == null ? null : o.toWay();
+            otherWay = o.toWay();
         } catch (ReflectiveOperationException e) {
             Logging.error(e);
             return -1;
         }
-        return o == null ? -1 : (equals(o) ? 0 : thisWay.compareTo(otherWay));
+        return equals(o) ? 0 : thisWay.compareTo(otherWay);
     }
 

trunk/src/org/openstreetmap/josm/data/osm/WaySegment.java

@@ -26 +26 @@
      * @param second second node
      * @return way segment
-     * @throws IllegalArgumentException if the node pair is not part of way
+     * @throws IllegalArgumentException if the node pair is not single a segment of the way
      */
     public static WaySegment forNodePair(Way way, Node first, Node second) {
@@ -32 +32 @@
         while (endIndex > 0) {
             final int indexOfFirst = way.getNodes().subList(0, endIndex).lastIndexOf(first);
+            if (indexOfFirst < 0)
+                break;
             if (second.equals(way.getNode(indexOfFirst + 1))) {
                 return new WaySegment(way, indexOfFirst);
@@ -37 +39 @@
             endIndex--;
         }
-        throw new IllegalArgumentException("The node pair is not consecutive part of the way!");
+        throw new IllegalArgumentException(IWaySegment.NOT_A_SEGMENT);
     }
 

trunk/test/unit/org/openstreetmap/josm/data/osm/WaySegmentTest.java

@@ -21 +21 @@
         final Node n3 = new Node(new LatLon(2, 0));
         final Node n4 = new Node(new LatLon(3, 0));
-        final Way w = new Way();
-        for (OsmPrimitive p : Arrays.asList(n1, n2, n3, n4, w)) {
+        final Way w1 = new Way();
+        final Way w2 = new Way();
+        for (OsmPrimitive p : Arrays.asList(n1, n2, n3, n4, w1, w2)) {
             ds.addPrimitive(p);
         }
-        w.addNode(n1);
-        w.addNode(n2);
-        w.addNode(n1);
-        w.addNode(n3);
-        w.addNode(n1);
-        w.addNode(n4);
-        w.addNode(n1);
-        assertEquals(WaySegment.forNodePair(w, n1, n2).getLowerIndex(), 0);
-        assertEquals(WaySegment.forNodePair(w, n1, n3).getLowerIndex(), 2);
-        assertEquals(WaySegment.forNodePair(w, n1, n4).getLowerIndex(), 4);
-        assertEquals(WaySegment.forNodePair(w, n4, n1).getLowerIndex(), 5);
-        IllegalArgumentException iae = assertThrows(IllegalArgumentException.class, () -> WaySegment.forNodePair(w, n3, n4));
-        assertEquals("The node pair is not consecutive part of the way!", iae.getMessage());
+        w1.addNode(n1);
+        w1.addNode(n2);
+        w1.addNode(n1);
+        w1.addNode(n3);
+        w1.addNode(n1);
+        w1.addNode(n4);
+        w1.addNode(n1);
+
+        w2.addNode(n1);
+        w2.addNode(n2);
+        w2.addNode(n3);
+
+        assertEquals(0, WaySegment.forNodePair(w1, n1, n2).getLowerIndex());
+        assertEquals(2, WaySegment.forNodePair(w1, n1, n3).getLowerIndex());
+        assertEquals(4, WaySegment.forNodePair(w1, n1, n4).getLowerIndex());
+        assertEquals(5, WaySegment.forNodePair(w1, n4, n1).getLowerIndex());
+        // two segments between n3 and n4
+        IllegalArgumentException iae = assertThrows(IllegalArgumentException.class, () -> WaySegment.forNodePair(w1, n3, n4));
+        assertEquals(IWaySegment.NOT_A_SEGMENT, iae.getMessage());
+        // wrong order
+        iae = assertThrows(IllegalArgumentException.class, () -> WaySegment.forNodePair(w2, n2, n1));
+        assertEquals(IWaySegment.NOT_A_SEGMENT, iae.getMessage());
+        // node is not in way
+        iae = assertThrows(IllegalArgumentException.class, () -> WaySegment.forNodePair(w2, n1, n4));
+        assertEquals(IWaySegment.NOT_A_SEGMENT, iae.getMessage());
     }
 }