Changeset 11177 in josm

Timestamp:

2016-10-27T00:32:55+02:00 (8 years ago)

Author:

simon04

Message:

Refactor OsmPrimitiveComparator

Replace one big comparator with functions to obtain specific simple comparators.

Location:

trunk

Files:

• src/org/openstreetmap/josm/data/APIDataSet.java (modified) (2 diffs)
• src/org/openstreetmap/josm/data/osm/OsmPrimitiveComparator.java (modified) (1 diff)
• src/org/openstreetmap/josm/gui/dialogs/InspectPrimitiveDialog.java (modified) (1 diff)
• src/org/openstreetmap/josm/gui/dialogs/SelectionListDialog.java (modified) (1 diff)
• src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java (modified) (2 diffs)
• test/unit/org/openstreetmap/josm/gui/dialogs/properties/PropertiesDialogTest.java (modified) (1 diff)

trunk/src/org/openstreetmap/josm/data/APIDataSet.java

@@ -21 +21 @@
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
+import org.openstreetmap.josm.data.osm.OsmPrimitiveComparator;
 import org.openstreetmap.josm.data.osm.PrimitiveId;
 import org.openstreetmap.josm.data.osm.Relation;
@@ -76 +77 @@
             }
         }
-        final Comparator<OsmPrimitive> orderingNodesWaysRelations = Comparator.comparingInt(osm -> osm.getType().ordinal());
-        final Comparator<OsmPrimitive> byUniqueId = Comparator.comparing(OsmPrimitive::getUniqueId);
+        final Comparator<OsmPrimitive> orderingNodesWaysRelations = OsmPrimitiveComparator.orderingNodesWaysRelations();
+        final Comparator<OsmPrimitive> byUniqueId = OsmPrimitiveComparator.comparingUniqueId();
         toAdd.sort(orderingNodesWaysRelations.thenComparing(byUniqueId));
         toUpdate.sort(orderingNodesWaysRelations.thenComparing(byUniqueId));

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

@@ -2 +2 @@
 package org.openstreetmap.josm.data.osm;
 
-import java.io.Serializable;
+import static java.util.Comparator.comparing;
+import static java.util.Comparator.comparingInt;
+
 import java.util.Comparator;
 import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Function;
 
 import org.openstreetmap.josm.gui.DefaultNameFormatter;
+import org.openstreetmap.josm.tools.AlphanumComparator;
 
 /**
- * Comparator, comparing pritimives by:<ul>
- * <li>type and ids in "quick" mode</li>
- * <li>type and objects display names instead</li>
- * </ul>
- * @since 4113
+ * Comparators for comparing primitives.
  */
-public class OsmPrimitiveComparator implements Comparator<OsmPrimitive>, Serializable {
-
-    private static final long serialVersionUID = 1L;
-
-    private final HashMap<OsmPrimitive, String> cache = new HashMap<>();
-    private final boolean relationsFirst;
-    private final boolean quick;
+public final class OsmPrimitiveComparator {
 
     /**
-     * Constructs a new {@code OsmPrimitiveComparator}.
+     * Returns a comparator comparing primitives by their name using {@link DefaultNameFormatter}.
+     *
+     * {@linkplain DefaultNameFormatter#format(OsmPrimitive) Formatted names} are cached.
+     *
+     * @return a comparator comparing primitives by their name using {@link DefaultNameFormatter}
      */
-    public OsmPrimitiveComparator() {
-        this(false, false);
+    public static Comparator<OsmPrimitive> comparingNames() {
+        final Comparator<String> digitsLast = comparing(str -> Character.isDigit(str.charAt(0)) ? 1 : 0);
+        return comparing(memoize(DefaultNameFormatter.getInstance()::format),
+                digitsLast.thenComparing(AlphanumComparator.getInstance()));
     }
 
     /**
-     * Constructs a new {@code OsmPrimitiveComparator}.
-     * @param quick if {@code true}, sorts by type and ids (fast), otherwise sort by type and display names (slower)
-     * @param relationsFirst if {@code true}, always list relations first
+     * Returns a comparator comparing primitives by their {@linkplain OsmPrimitive#getUniqueId unique id}.
+     *
+     * @return a comparator comparing primitives by their {@linkplain OsmPrimitive#getUniqueId unique id}.
      */
-    public OsmPrimitiveComparator(boolean quick, boolean relationsFirst) {
-        this.quick = quick;
-        this.relationsFirst = relationsFirst;
+    public static Comparator<OsmPrimitive> comparingUniqueId() {
+        return comparing(OsmPrimitive::getUniqueId);
     }
 
-    private String cachedName(OsmPrimitive p) {
-        String name = cache.get(p);
-        if (name == null) {
-            name = p.getDisplayName(DefaultNameFormatter.getInstance());
-            cache.put(p, name);
-        }
-        return name;
+    /**
+     * Returns a comparator ordering the primitives by type in the order NODE, WAY, RELATION
+     *
+     * @return a comparator ordering the primitives by type in the order NODE, WAY, RELATION
+     */
+    public static Comparator<OsmPrimitive> orderingNodesWaysRelations() {
+        return comparingInt(osm -> osm.getType().ordinal());
     }
 
-    private int compareName(OsmPrimitive a, OsmPrimitive b) {
-        String an = cachedName(a);
-        String bn = cachedName(b);
-        // make sure display names starting with digits are the end of the list
-        if (Character.isDigit(an.charAt(0)) && Character.isDigit(bn.charAt(0)))
-            return an.compareTo(bn);
-        else if (Character.isDigit(an.charAt(0)) && !Character.isDigit(bn.charAt(0)))
-            return 1;
-        else if (!Character.isDigit(an.charAt(0)) && Character.isDigit(bn.charAt(0)))
-            return -1;
-        return an.compareTo(bn);
+    /**
+     * Returns a comparator ordering the primitives by type in the order WAY, RELATION, NODE
+     *
+     * @return a comparator ordering the primitives by type in the order WAY, RELATION, NODE
+     */
+    public static Comparator<OsmPrimitive> orderingWaysRelationsNodes() {
+        return comparingInt(osm -> {
+            switch (osm.getType()) {
+                case WAY:
+                    return 1;
+                case RELATION:
+                    return 2;
+                case NODE:
+                    return 3;
+                default:
+                    throw new IllegalStateException();
+            }
+        });
     }
 
-    private static int compareId(OsmPrimitive a, OsmPrimitive b) {
-        long idA = a.getUniqueId();
-        long idB = b.getUniqueId();
-        if (idA < idB) return -1;
-        if (idA > idB) return 1;
-        return 0;
+    private static <T, R> Function<T, R> memoize(Function<T, R> base) {
+        final Map<T, R> cache = new HashMap<>();
+        return t -> cache.computeIfAbsent(t, base);
     }
 
-    private int compareType(OsmPrimitive a, OsmPrimitive b) {
-        if (relationsFirst) {
-            // show relations before ways, then nodes
-            if (a.getType().equals(OsmPrimitiveType.RELATION)) return -1;
-            if (a.getType().equals(OsmPrimitiveType.NODE)) return 1;
-            // a is a way
-            if (b.getType().equals(OsmPrimitiveType.RELATION)) return 1;
-            // b is a node
-        } else {
-            // show ways before relations, then nodes
-            if (a.getType().equals(OsmPrimitiveType.WAY)) return -1;
-            if (a.getType().equals(OsmPrimitiveType.NODE)) return 1;
-            // a is a relation
-            if (b.getType().equals(OsmPrimitiveType.WAY)) return 1;
-            // b is a node
-        }
-        return -1;
-    }
-
-    @Override
-    public int compare(OsmPrimitive a, OsmPrimitive b) {
-        if (a.getType().equals(b.getType()))
-            return quick ? compareId(a, b) : compareName(a, b);
-        return compareType(a, b);
+    private OsmPrimitiveComparator() {
     }
 }

trunk/src/org/openstreetmap/josm/gui/dialogs/InspectPrimitiveDialog.java

@@ -107 +107 @@
     protected static String buildDataText(OsmDataLayer layer, List<OsmPrimitive> primitives) {
         InspectPrimitiveDataText dt = new InspectPrimitiveDataText(layer);
-        primitives.sort(new OsmPrimitiveComparator());
-        for (OsmPrimitive o : primitives) {
-            dt.addPrimitive(o);
-        }
+        primitives.stream()
+                .sorted(OsmPrimitiveComparator.orderingWaysRelationsNodes().thenComparing(OsmPrimitiveComparator.comparingNames()))
+                .forEachOrdered(dt::addPrimitive);
         return dt.toString();
     }

trunk/src/org/openstreetmap/josm/gui/dialogs/SelectionListDialog.java

@@ -659 +659 @@
             if (selection.size() <= Main.pref.getInteger("selection.no_sort_above", 100_000)) {
                 boolean quick = selection.size() > Main.pref.getInteger("selection.fast_sort_above", 10_000);
-                selection.sort(new OsmPrimitiveComparator(quick, false));
+                selection.sort(OsmPrimitiveComparator.orderingWaysRelationsNodes().thenComparing(quick
+                        ? OsmPrimitiveComparator.comparingUniqueId()
+                        : OsmPrimitiveComparator.comparingNames()));
             }
         }

trunk/src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java

@@ -646 +646 @@
          * (Only works if the data layer has not been saved to and been loaded from an osm file before.)
          */
-        final List<Way> sortedWays = new ArrayList<>(ways);
-        sortedWays.sort(new OsmPrimitiveComparator(true, false)); // sort by OsmPrimitive#getUniqueId ascending
-        Collections.reverse(sortedWays); // sort by OsmPrimitive#getUniqueId descending
-        for (Way w : sortedWays) {
+        ways.stream()
+                .sorted(OsmPrimitiveComparator.comparingUniqueId().reversed())
+                .forEachOrdered(w -> {
             if (!w.isUsable()) {
-                continue;
+                return;
             }
             Collection<Collection<WayPoint>> trk = new ArrayList<>();
@@ -677 +676 @@
 
             gpxData.tracks.add(new ImmutableGpxTrack(trk, trkAttr));
-        }
+        });
     }
 

trunk/test/unit/org/openstreetmap/josm/gui/dialogs/properties/PropertiesDialogTest.java

@@ -34 +34 @@
     private static String createSearchSetting(DataSet ds, boolean sameType) {
         List<OsmPrimitive> sel = new ArrayList<>(ds.allPrimitives());
-        Collections.sort(sel, new OsmPrimitiveComparator(true, false));
+        Collections.sort(sel, OsmPrimitiveComparator.comparingUniqueId());
         return PropertiesDialog.createSearchSetting("foo", sel, sameType).text;
     }