Index: trunk/src/org/openstreetmap/josm/data/APIDataSet.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/APIDataSet.java (revision 11176) +++ trunk/src/org/openstreetmap/josm/data/APIDataSet.java (revision 11177) @@ -21,4 +21,5 @@ 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,6 +77,6 @@ } } - final Comparator orderingNodesWaysRelations = Comparator.comparingInt(osm -> osm.getType().ordinal()); - final Comparator byUniqueId = Comparator.comparing(OsmPrimitive::getUniqueId); + final Comparator orderingNodesWaysRelations = OsmPrimitiveComparator.orderingNodesWaysRelations(); + final Comparator byUniqueId = OsmPrimitiveComparator.comparingUniqueId(); toAdd.sort(orderingNodesWaysRelations.thenComparing(byUniqueId)); toUpdate.sort(orderingNodesWaysRelations.thenComparing(byUniqueId)); Index: trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitiveComparator.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitiveComparator.java (revision 11176) +++ trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitiveComparator.java (revision 11177) @@ -2,96 +2,77 @@ 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:
    - *
  • type and ids in "quick" mode
    • - *
  • type and objects display names instead
    • - *
- * @since 4113 + * Comparators for comparing primitives. */ -public class OsmPrimitiveComparator implements Comparator, Serializable { - - private static final long serialVersionUID = 1L; - - private final HashMap 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 comparingNames() { + final Comparator 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 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 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 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 Function memoize(Function base) { + final Map 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() { } } Index: trunk/src/org/openstreetmap/josm/gui/dialogs/InspectPrimitiveDialog.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/dialogs/InspectPrimitiveDialog.java (revision 11176) +++ trunk/src/org/openstreetmap/josm/gui/dialogs/InspectPrimitiveDialog.java (revision 11177) @@ -107,8 +107,7 @@ protected static String buildDataText(OsmDataLayer layer, List 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(); } Index: trunk/src/org/openstreetmap/josm/gui/dialogs/SelectionListDialog.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/dialogs/SelectionListDialog.java (revision 11176) +++ trunk/src/org/openstreetmap/josm/gui/dialogs/SelectionListDialog.java (revision 11177) @@ -659,5 +659,7 @@ 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())); } } Index: trunk/src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java (revision 11176) +++ trunk/src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java (revision 11177) @@ -646,10 +646,9 @@ * (Only works if the data layer has not been saved to and been loaded from an osm file before.) */ - final List 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> trk = new ArrayList<>(); @@ -677,5 +676,5 @@ gpxData.tracks.add(new ImmutableGpxTrack(trk, trkAttr)); - } + }); } Index: trunk/test/unit/org/openstreetmap/josm/gui/dialogs/properties/PropertiesDialogTest.java =================================================================== --- trunk/test/unit/org/openstreetmap/josm/gui/dialogs/properties/PropertiesDialogTest.java (revision 11176) +++ trunk/test/unit/org/openstreetmap/josm/gui/dialogs/properties/PropertiesDialogTest.java (revision 11177) @@ -34,5 +34,5 @@ private static String createSearchSetting(DataSet ds, boolean sameType) { List sel = new ArrayList<>(ds.allPrimitives()); - Collections.sort(sel, new OsmPrimitiveComparator(true, false)); + Collections.sort(sel, OsmPrimitiveComparator.comparingUniqueId()); return PropertiesDialog.createSearchSetting("foo", sel, sameType).text; }