[josm-dev] [PATCH 18/24] clean up MergeNodeAction

Dave Hansen dave at sr71.net
Sat May 3 20:15:09 BST 2008


---

 core-dave/src/org/openstreetmap/josm/actions/MergeNodesAction.java |   71 +++-------
 core-dave/src/org/openstreetmap/josm/data/osm/Relation.java        |   33 ++++
 2 files changed, 61 insertions(+), 43 deletions(-)

diff -puN src/org/openstreetmap/josm/actions/MergeNodesAction.java~reversewaycommand src/org/openstreetmap/josm/actions/MergeNodesAction.java
--- core/src/org/openstreetmap/josm/actions/MergeNodesAction.java~reversewaycommand	2008-05-03 12:08:44.000000000 -0700
+++ core-dave/src/org/openstreetmap/josm/actions/MergeNodesAction.java	2008-05-03 12:08:44.000000000 -0700
@@ -11,6 +11,7 @@ import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedList;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeMap;
@@ -24,9 +25,11 @@ import javax.swing.JOptionPane;
 import javax.swing.JPanel;
 
 import org.openstreetmap.josm.Main;
-import org.openstreetmap.josm.command.ChangeCommand;
+import org.openstreetmap.josm.command.ChangePropertyCommand;
 import org.openstreetmap.josm.command.Command;
 import org.openstreetmap.josm.command.DeleteCommand;
+import org.openstreetmap.josm.command.RemoveNodeInWayCommand;
+import org.openstreetmap.josm.command.ReplaceNodeInWayCommand;
 import org.openstreetmap.josm.command.SequenceCommand;
 import org.openstreetmap.josm.data.SelectionChangedListener;
 import org.openstreetmap.josm.data.osm.DataSet;
@@ -99,8 +102,6 @@ public class MergeNodesAction extends Jo
 	 * really do the merging - returns the node that is left
 	 */
 	public static Node mergeNodes(LinkedList<Node> allNodes, Node dest) {
-		Node newNode = new Node(dest);
-
 		// Check whether all ways have identical relationship membership. More
 		// specifically: If one of the selected ways is a member of relation X
 		// in role Y, then all selected ways must be members of X in role Y.
@@ -161,6 +162,7 @@ public class MergeNodesAction extends Jo
 				props.get(e.getKey()).add(e.getValue());
 			}
 		}
+		LinkedList<Command> cmds = new LinkedList<Command>();
 
 		// display conflict dialog
 		Map<String, JComboBox> components = new HashMap<String, JComboBox>();
@@ -168,7 +170,7 @@ public class MergeNodesAction extends Jo
 		for (Entry<String, Set<String>> e : props.entrySet()) {
 			if (TigerUtils.isTigerTag(e.getKey())) {
 				String combined = TigerUtils.combineTags(e.getKey(), e.getValue());
-				newNode.put(e.getKey(), combined);
+				cmds.add(new ChangePropertyCommand(dest, e.getKey(), combined));
 			} else if (e.getValue().size() > 1) {
 				JComboBox c = new JComboBox(e.getValue().toArray());
 				c.setEditable(true);
@@ -177,20 +179,20 @@ public class MergeNodesAction extends Jo
 				p.add(c, GBC.eol());
 				components.put(e.getKey(), c);
 			} else
-				newNode.put(e.getKey(), e.getValue().iterator().next());
+				cmds.add(new ChangePropertyCommand(dest, e.getKey(), e
+				        .getValue().iterator().next()));
 		}
 
 		if (!components.isEmpty()) {
 			int answer = JOptionPane.showConfirmDialog(Main.parent, p, tr("Enter values for all conflicts."), JOptionPane.OK_CANCEL_OPTION);
 			if (answer != JOptionPane.OK_OPTION)
 				return null;
-			for (Entry<String, JComboBox> e : components.entrySet())
-				newNode.put(e.getKey(), e.getValue().getEditor().getItem().toString());
+			for (Entry<String, JComboBox> e : components.entrySet()) {
+				String newValue = e.getValue().getEditor().getItem().toString();
+				cmds.add(new ChangePropertyCommand(dest, e.getKey(), newValue));
+			}
 		}
 
-		LinkedList<Command> cmds = new LinkedList<Command>();
-		cmds.add(new ChangeCommand(dest, newNode));
-
 		Collection<OsmPrimitive> del = new HashSet<OsmPrimitive>();
 
 		for (Way w : Main.ds.ways) {
@@ -204,21 +206,23 @@ public class MergeNodesAction extends Jo
 			}
 			if (!modify) continue;
 			// OK - this way contains one or more nodes to change
-			ArrayList<Node> nn = new ArrayList<Node>();
+			int new_way_size = w.nodes.size();
 			Node lastNode = null;
 			for (int i = 0; i < w.nodes.size(); i++) {
 				Node pushNode = w.nodes.get(i);
-				if (allNodes.contains(pushNode)) {
-					pushNode = dest;
-				}
-				if (pushNode != lastNode) {
-					nn.add(pushNode);
+				if (!allNodes.contains(pushNode))
+					continue;
+				if (pushNode == lastNode) {
+					new_way_size--;
+					cmds.add(new RemoveNodeInWayCommand(w, pushNode));
+				} else {
+					cmds.add(new ReplaceNodeInWayCommand(w, pushNode, dest));
+					lastNode = pushNode;
 				}
-				lastNode = pushNode;
 			}
-			if (nn.size() < 2) {
-				CollectBackReferencesVisitor backRefs =
-					new CollectBackReferencesVisitor(Main.ds, false);
+			if (new_way_size < 2) {
+				CollectBackReferencesVisitor backRefs = 
+						new CollectBackReferencesVisitor(Main.ds, false);
 				w.visit(backRefs);
 				if (!backRefs.data.isEmpty()) {
 					JOptionPane.showMessageDialog(Main.parent,
@@ -227,11 +231,6 @@ public class MergeNodesAction extends Jo
 					return null;
 				}
 				del.add(w);
-			} else {
-				Way newWay = new Way(w);
-				newWay.nodes.clear();
-				newWay.nodes.addAll(nn);
-				cmds.add(new ChangeCommand(w, newWay));
 			}
 		}
 
@@ -241,24 +240,10 @@ public class MergeNodesAction extends Jo
 		if (!del.isEmpty()) cmds.add(new DeleteCommand(del));
 
 		// modify all relations containing the now-deleted nodes
-		for (Relation r : relationsUsingNodes) {
-			Relation newRel = new Relation(r);
-			newRel.members.clear();
-			HashSet<String> rolesToReAdd = new HashSet<String>();
-			for (RelationMember rm : r.members) {
-				// Don't copy the member if it points to one of our nodes,
-				// just keep a note to re-add it later on.
-				if (allNodes.contains(rm.member)) {
-					rolesToReAdd.add(rm.role);
-				} else {
-					newRel.members.add(rm);
-				}
-			}
-			for (String role : rolesToReAdd) {
-				newRel.members.add(new RelationMember(role, dest));
-			}
-			cmds.add(new ChangeCommand(r, newRel));
-		}
+		List<OsmPrimitive> allOsmPrimitives = new LinkedList<OsmPrimitive>();
+		allOsmPrimitives.addAll(allNodes);
+		for (Relation r : relationsUsingNodes)
+			cmds.add(r.updateMembers(allOsmPrimitives, dest));
 
 		Main.main.undoRedo.add(new SequenceCommand(tr("Merge {0} nodes", allNodes.size()), cmds));
 		Main.ds.setSelected(dest);
diff -puN src/org/openstreetmap/josm/data/osm/Relation.java~reversewaycommand src/org/openstreetmap/josm/data/osm/Relation.java
--- core/src/org/openstreetmap/josm/data/osm/Relation.java~reversewaycommand	2008-05-03 12:08:44.000000000 -0700
+++ core-dave/src/org/openstreetmap/josm/data/osm/Relation.java	2008-05-03 12:08:44.000000000 -0700
@@ -1,9 +1,16 @@
 package org.openstreetmap.josm.data.osm;
 
+import static org.openstreetmap.josm.tools.I18n.tr;
+
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.LinkedList;
 import java.util.List;
 
+import org.openstreetmap.josm.command.AddRelationMemberCommand;
+import org.openstreetmap.josm.command.Command;
+import org.openstreetmap.josm.command.RemoveRelationMemberCommand;
+import org.openstreetmap.josm.command.SequenceCommand;
 import org.openstreetmap.josm.data.osm.visitor.Visitor;
 
 /**
@@ -73,4 +80,30 @@ public final class Relation extends OsmP
 				return true;
 		return false;
 	}
+
+	/**
+	 * @param from - the list of relation members that will get changed
+	 * @param to - relation member that will be the new target
+	 *
+	 * If any RelationMember is in @from, it will get updated to point
+	 * to @to.  If any RelationMember is no
+	 *
+	 * @return set of commands to perform the update
+	 */
+	public Command updateMembers(List<OsmPrimitive> _from, OsmPrimitive to) {
+		List<OsmPrimitive> from = new LinkedList<OsmPrimitive>(_from);
+		List<Command> cmds = new LinkedList<Command>();
+		if (from.contains(to))
+			from.remove(to);
+		for (RelationMember rm : this.members) {
+			// Don't copy the member if it to one of our ways, just keep a
+			// note to re-add it later on.
+			if (from.contains(rm.member)) {
+				RelationMember newrm = new RelationMember(rm.role, to);
+				cmds.add(new RemoveRelationMemberCommand(this, rm));
+				cmds.add(new AddRelationMemberCommand(this, newrm));
+			}
+		}
+		return new SequenceCommand(tr("Update relation members"), cmds);
+	}
 }
_




More information about the josm-dev mailing list