[josm-dev] [PATCH] Relation member sort cleanup

[Christiaan Welvaart]

hi,

Since the patch I mentioned in my comment on 
http://josm.openstreetmap.de/ticket/2789 was only a draft, now that it was 
checked in with changeset 1748 it needs to be cleaned up a bit. The 
attached patch contains the following:
- relation member sort fixes:
   o drop bogus compare-by-value TreeMap, use a HashMap instead
   o remove diagnostic message for each sort step
   o do not emit a message when the last element could not be 'sorted'


It was mentioned that the sort feature is not really useful. Why is that, 
and how could it be made (more) useful?


     Christiaan
-------------- next part --------------
Index: org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java
===================================================================
--- org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java	(revision 1749)
+++ org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java	(working copy)
@@ -326,37 +326,11 @@
         return buttonPanel;
     }
 
-    private class NodeCompare implements java.util.Comparator<Node>
-    {
-        public int compare(Node a, Node b)
-        {
-            int result = 0;
-
-            if ((a.id == 0) && (b.id == 0))
-            {
-                if (a.getCoor().lat() == b.getCoor().lat())
-                {
-                    result = Double.compare(a.getCoor().lon(), b.getCoor().lon());
-                }
-                else
-                {
-                    result = Double.compare(a.getCoor().lat(), b.getCoor().lat());
-                }
-            }
-            else
-            {
-                result = a.compareTo(b);
-            }
-
-            return result;
-        }
-    }
-
     private void sort() {
-        java.util.TreeMap<Node, java.util.TreeSet<Integer>>   points =
-            new java.util.TreeMap<Node, java.util.TreeSet<Integer>>(new NodeCompare());
-        java.util.TreeMap<Node, Integer>   nodes =
-            new java.util.TreeMap<Node, Integer>(new NodeCompare());
+        java.util.HashMap<Node, java.util.TreeSet<Integer>>   points =
+            new java.util.HashMap<Node, java.util.TreeSet<Integer>>();
+        java.util.HashMap<Node, Integer>   nodes =
+            new java.util.HashMap<Node, Integer>();
         int                                i;
         boolean                            lastWayStartUsed = true;
 
@@ -445,11 +419,10 @@
             } catch(NullPointerException f) {}
             catch(java.util.NoSuchElementException e) {}
 
-            if (m2 == null)
+            if ((m2 == null) && ((i+1) < clone.members.size()))
             {
                 // TODO: emit some message that sorting failed
                 System.err.println("relation member sort: could not find linked way or node for member " + i);
-                System.err.println("last way start used = " + lastWayStartUsed);
                 break;
             }
 
@@ -463,60 +436,59 @@
                 catch(ClassCastException e)
                 {
                 }
-            }
 
-            if ((m2 < clone.members.size()) && ((i+1) < clone.members.size()))
-            {
-                RelationMember  a = clone.members.get(i+1);
-                RelationMember  b = clone.members.get(m2);
-
-                if (m2 != (i+1))
+                if ((m2 < clone.members.size()) && ((i+1) < clone.members.size()))
                 {
-                    System.err.println("swapping " + (i+1) + " and " + m2);
-                    clone.members.set(i+1, b);
-                    clone.members.set(m2, a);
+                    RelationMember  a = clone.members.get(i+1);
+                    RelationMember  b = clone.members.get(m2);
 
-                    try
+                    if (m2 != (i+1))
                     {
-                        if (!points.get(((Way)b.member).firstNode()).remove(m2))
+                        clone.members.set(i+1, b);
+                        clone.members.set(m2, a);
+
+                        try
                         {
-                            System.err.println("relation member sort: could not remove start mapping for " + m2);
+                            if (!points.get(((Way)b.member).firstNode()).remove(m2))
+                            {
+                                System.err.println("relation member sort: could not remove start mapping for " + m2);
+                            }
+                            if (!points.get(((Way)b.member).lastNode()).remove(m2))
+                            {
+                                System.err.println("relation member sort: could not remove end mapping for " + m2);
+                            }
                         }
-                        if (!points.get(((Way)b.member).lastNode()).remove(m2))
+                        catch(ClassCastException e1)
                         {
-                            System.err.println("relation member sort: could not remove end mapping for " + m2);
+                            nodes.remove(b.member);
                         }
-                    }
-                    catch(ClassCastException e1)
-                    {
-                        nodes.remove(b.member);
-                    }
 
+                        try
+                        {
+                            points.get(((Way)a.member).firstNode()).add(m2);
+                            points.get(((Way)a.member).lastNode()).add(m2);
+                        }
+                        catch(ClassCastException e1)
+                        {
+                            nodes.put((Node)a.member, m2);
+                        }
+                    }
                     try
                     {
-                        points.get(((Way)a.member).firstNode()).add(m2);
-                        points.get(((Way)a.member).lastNode()).add(m2);
+                        if (!points.get(((Way)a.member).firstNode()).remove(i+1))
+                        {
+                            System.err.println("relation member sort: could not remove start mapping for " + (i+1));
+                        }
+                        if (!points.get(((Way)a.member).lastNode()).remove(i+1))
+                        {
+                            System.err.println("relation member sort: could not remove end mapping for " + (i+1));
+                        }
                     }
                     catch(ClassCastException e1)
                     {
-                        nodes.put((Node)a.member, m2);
+                        nodes.remove(a.member);
                     }
                 }
-                try
-                {
-                    if (!points.get(((Way)a.member).firstNode()).remove(i+1))
-                    {
-                        System.err.println("relation member sort: could not remove start mapping for " + (i+1));
-                    }
-                    if (!points.get(((Way)a.member).lastNode()).remove(i+1))
-                    {
-                        System.err.println("relation member sort: could not remove end mapping for " + (i+1));
-                    }
-                }
-                catch(ClassCastException e1)
-                {
-                    nodes.remove(a.member);
-                }
             }
         }