Skip to content

Commit

Permalink
Fix #23252: Ignore line_management=transition|termination in `Uncon…
Browse files Browse the repository at this point in the history 
…nectedPowerTest` (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

git-svn-id: https://josm.openstreetmap.de/svn/trunk@18885 0c6e7542-c601-0410-84e7-c038aed88b3b
  • Loading branch information
taylor.smock committed Oct 30, 2023
1 parent edf0cb9 commit 49697c0
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 25 deletions.
33 changes: 20 additions & 13 deletions src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public abstract class AbstractPrimitive implements IPrimitive, IFilterablePrimit
* filter mechanism (i.e. FLAG_DISABLED is set).
* 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).
*/
Expand Down Expand Up @@ -520,10 +520,10 @@ public TagMap getKeys() {

@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]);
}
}
}
Expand Down Expand Up @@ -753,17 +753,17 @@ public final int getNumKeys() {

@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]);
if (tKeys.length == 2) {
return Collections.singleton(tKeys[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]);
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;
}
Expand Down Expand Up @@ -809,7 +809,14 @@ public boolean hasKey(String key) {
* @since 11587
*/
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;
}

/**
Expand Down
17 changes: 14 additions & 3 deletions src/org/openstreetmap/josm/data/osm/Tagged.java
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.data.osm;

import java.util.Arrays;
import java.util.Collection;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -146,7 +145,13 @@ default boolean hasTag(String key, String value) {
* @since 13668
*/
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;
}

/**
Expand Down Expand Up @@ -181,7 +186,13 @@ default boolean hasTagDifferent(String key, String value) {
* @since 13668
*/
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;
}

/**
Expand Down
23 changes: 14 additions & 9 deletions src/org/openstreetmap/josm/data/validation/tests/UnconnectedWays.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@
* @author frsantos
*/
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;

Expand Down Expand Up @@ -101,7 +105,7 @@ protected boolean ignoreUnconnectedEndNode(Node n) {
|| n.hasTag("amenity", "parking_entrance", "ferry_terminal")
|| 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));
}
}

Expand All @@ -119,10 +123,10 @@ public UnconnectedRailways() {

@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");
}

@Override
Expand Down Expand Up @@ -196,12 +200,13 @@ public UnconnectedPower() {

@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");
}
}

Expand Down Expand Up @@ -269,11 +274,11 @@ protected Map<Node, MyWaySegment> getHighwayEndNodesNearOtherHighway() {
map.clear();
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
&& !s.isConnectedTo(endnode) && !s.obstacleBetween(endnode)) {
Expand All @@ -295,7 +300,7 @@ protected Map<Node, MyWaySegment> getWayEndNodesNearOtherWay() {
if (!s.concernsArea) {
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);
if (otherWay != null) {
Expand Down

0 comments on commit 49697c0

Please sign in to comment.