Changeset 18885 in josm

Timestamp:

2023-10-30T20:12:12+01:00 (7 months ago)

Author:

taylor.smock

Message:

Fix #23252: Ignore line_management=transition|termination in UnconnectedPowerTest (patch by gaben, modified)

Modifications are as follows:

• Lint fixes
• Performance enhancements (profiling done with validator and an overpass download of Mesa County, Colorado)
  • Iterating over an array is much more efficient than using streams. For methods that may be frequently called, using Arrays.stream is currently not a good idea.
    • hasTagDifferent: 2.09 MB -> 0
    • hasTag: 125MB -> 0
    • hasKey: 879MB -> 0

Location:

trunk/src/org/openstreetmap/josm/data

Files:

• osm/AbstractPrimitive.java (modified) (4 diffs)
• osm/Tagged.java (modified) (3 diffs)
• validation/tests/UnconnectedWays.java (modified) (6 diffs)

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

@@ -77 +77 @@
      * Then it indicates, whether it is completely hidden or
      * just shown in gray color.
-     *
+     * <p>
      * When the primitive is not disabled, this flag should be
      * unset as well (for efficient access).
@@ -521 +521 @@
     @Override
     public void visitKeys(KeyValueVisitor visitor) {
-        String[] keys = this.keys;
-        if (keys != null) {
-            for (int i = 0; i < keys.length; i += 2) {
-                visitor.visitKeyValue(this, keys[i], keys[i + 1]);
+        String[] tKeys = this.keys;
+        if (tKeys != null) {
+            for (int i = 0; i < tKeys.length; i += 2) {
+                visitor.visitKeyValue(this, tKeys[i], tKeys[i + 1]);
             }
         }
@@ -754 +754 @@
     @Override
     public final Collection<String> keySet() {
-        String[] keys = this.keys;
-        if (keys == null) {
+        String[] tKeys = this.keys;
+        if (tKeys == null) {
             return Collections.emptySet();
         }
-        if (keys.length == 2) {
-            return Collections.singleton(keys[0]);
-        }
-
-        final Set<String> result = new HashSet<>(Utils.hashMapInitialCapacity(keys.length / 2));
-        for (int i = 0; i < keys.length; i += 2) {
-            result.add(keys[i]);
+        if (tKeys.length == 2) {
+            return Collections.singleton(tKeys[0]);
+        }
+
+        final Set<String> result = new HashSet<>(Utils.hashMapInitialCapacity(tKeys.length / 2));
+        for (int i = 0; i < tKeys.length; i += 2) {
+            result.add(tKeys[i]);
         }
         return result;
@@ -810 +810 @@
      */
     public boolean hasKey(String... keys) {
-        return keys != null && Arrays.stream(keys).anyMatch(this::hasKey);
+        if (keys != null) {
+            for (String key : keys) {
+                if (this.hasKey(key)) {
+                    return true;
+                }
+            }
+        }
+        return false;
     }
 

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

@@ -2 +2 @@
 package org.openstreetmap.josm.data.osm;
 
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Map;
@@ -147 +146 @@
      */
     default boolean hasTag(String key, String... values) {
-        return hasTag(key, Arrays.asList(values));
+        final String actualValue = get(key);
+        for (String value : values) {
+            if (Objects.equals(value, actualValue)) {
+                return true;
+            }
+        }
+        return false;
     }
 
@@ -182 +187 @@
      */
     default boolean hasTagDifferent(String key, String... values) {
-        return hasTagDifferent(key, Arrays.asList(values));
+        final String actual = get(key);
+        for (String value : values) {
+            if (Objects.equals(actual, value)) {
+                return false;
+            }
+        }
+        return true;
     }
 

trunk/src/org/openstreetmap/josm/data/validation/tests/UnconnectedWays.java

@@ -53 +53 @@
  */
 public abstract class UnconnectedWays extends Test {
+    private static final String POWER = "power";
+    private static final String PLATFORM = "platform";
+    private static final String PLATFORM_EDGE = "platform_edge";
+    private static final String CONSTRUCTION = "construction";
     private final int code;
     private final boolean isHighwayTest;
@@ -102 +106 @@
                     || n.isKeyTrue("noexit")
                     || n.hasKey("entrance", "barrier")
-                    || n.getParentWays().stream().anyMatch(p -> isBuilding(p) || p.hasTag(RAILWAY, "platform", "platform_edge"));
+                    || n.getParentWays().stream().anyMatch(p -> isBuilding(p) || p.hasTag(RAILWAY, PLATFORM, PLATFORM_EDGE));
         }
     }
@@ -120 +124 @@
         @Override
         protected boolean isCandidate(OsmPrimitive p) {
-            if (p.hasTag(RAILWAY, "construction") && p.hasKey("construction"))
-                return p.hasTagDifferent("construction", "platform", "platform_edge", "service_station", "station");
+            if (p.hasTag(RAILWAY, CONSTRUCTION) && p.hasKey(CONSTRUCTION))
+                return p.hasTagDifferent(CONSTRUCTION, PLATFORM, PLATFORM_EDGE, "service_station", "station");
             return p.hasTagDifferent(RAILWAY, "proposed", "planned", "abandoned", "razed", "disused", "no",
-                    "platform", "platform_edge", "service_station", "station");
+                    PLATFORM, PLATFORM_EDGE, "service_station", "station");
         }
 
@@ -197 +201 @@
         @Override
         protected boolean isCandidate(OsmPrimitive p) {
-            return p.hasTag("power", "line", "minor_line", "cable");
+            return p.hasTag(POWER, "line", "minor_line", "cable");
         }
 
         @Override
         protected boolean ignoreUnconnectedEndNode(Node n) {
-            return n.hasTag("power", "terminal") || n.hasTag("location:transition", "yes");
+            return n.hasTag(POWER, "terminal") || n.hasTag("location:transition", "yes")
+                    || n.hasTag("line_management", "transition", "termination");
         }
     }
@@ -270 +275 @@
                 return map;
             }
-            if (s.w.hasTag(HIGHWAY, "platform"))
+            if (s.w.hasTag(HIGHWAY, PLATFORM))
                 continue;
             for (Node endnode : s.nearbyNodes(mindist)) {
                 Way parentWay = getWantedParentWay(endnode);
-                if (parentWay != null && !parentWay.hasTag(HIGHWAY, "platform")
+                if (parentWay != null && !parentWay.hasTag(HIGHWAY, PLATFORM)
                         && Objects.equals(OsmUtils.getLayer(s.w), OsmUtils.getLayer(parentWay))
                         // to handle intersections of 't' shapes and similar
@@ -296 +301 @@
                 for (Node endnode : s.nearbyNodes(mindist)) {
                     if (!s.isConnectedTo(endnode)) {
-                        if (s.w.hasTag("power")) {
+                        if (s.w.hasTag(POWER)) {
                             boolean badConnection = false;
                             Way otherWay = getWantedParentWay(endnode);