Index: trunk/src/org/openstreetmap/josm/actions/UnJoinNodeWayAction.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/UnJoinNodeWayAction.java (revision 15012) +++ trunk/src/org/openstreetmap/josm/actions/UnJoinNodeWayAction.java (revision 15013) @@ -10,4 +10,5 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.HashSet; import java.util.LinkedList; import java.util.List; @@ -89,5 +90,6 @@ // I'm sure there's a better way to handle this - UndoRedoHandler.getInstance().add(new RemoveNodesCommand(selectedWay, selectedNodes)); + UndoRedoHandler.getInstance().add( + new RemoveNodesCommand(selectedWay, new HashSet<>(selectedNodes))); } Index: trunk/src/org/openstreetmap/josm/command/AbstractNodesCommand.java =================================================================== --- trunk/src/org/openstreetmap/josm/command/AbstractNodesCommand.java (revision 15013) +++ trunk/src/org/openstreetmap/josm/command/AbstractNodesCommand.java (revision 15013) @@ -0,0 +1,84 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.command; + +import java.util.Collection; +import java.util.Objects; + +import javax.swing.Icon; + +import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.data.osm.Node; +import org.openstreetmap.josm.data.osm.OsmPrimitive; +import org.openstreetmap.josm.data.osm.OsmPrimitiveType; +import org.openstreetmap.josm.data.osm.Way; +import org.openstreetmap.josm.tools.ImageProvider; + +/** + * Abstracts superclass of {@link ChangeNodesCommand} / {@link RemoveNodesCommand}. + * @param type of nodes collection used for this command + * @since 15013 + */ +public abstract class AbstractNodesCommand> extends Command { + + protected final Way way; + protected final C cmdNodes; + + /** + * Constructs a new {@code AbstractNodesCommand}. + * @param way The way to modify + * @param cmdNodes The collection of nodes for this command + */ + protected AbstractNodesCommand(Way way, C cmdNodes) { + this(way.getDataSet(), way, cmdNodes); + } + + /** + * Constructs a new {@code AbstractNodesCommand}. + * @param ds The target data set. Must not be {@code null} + * @param way The way to modify + * @param cmdNodes The collection of nodes for this command + */ + protected AbstractNodesCommand(DataSet ds, Way way, C cmdNodes) { + super(ds); + this.way = Objects.requireNonNull(way, "way"); + this.cmdNodes = Objects.requireNonNull(cmdNodes, "cmdNodes"); + if (cmdNodes.isEmpty()) { + throw new IllegalArgumentException("Nodes collection is empty"); + } + } + + protected abstract void modifyWay(); + + @Override + public boolean executeCommand() { + super.executeCommand(); + modifyWay(); + way.setModified(true); + return true; + } + + @Override + public void fillModifiedData(Collection modified, Collection deleted, Collection added) { + modified.add(way); + } + + @Override + public Icon getDescriptionIcon() { + return ImageProvider.get(OsmPrimitiveType.WAY); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), way, cmdNodes); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null || getClass() != obj.getClass()) return false; + if (!super.equals(obj)) return false; + AbstractNodesCommand that = (AbstractNodesCommand) obj; + return Objects.equals(way, that.way) && + Objects.equals(cmdNodes, that.cmdNodes); + } +} Index: trunk/src/org/openstreetmap/josm/command/ChangeNodesCommand.java =================================================================== --- trunk/src/org/openstreetmap/josm/command/ChangeNodesCommand.java (revision 15012) +++ trunk/src/org/openstreetmap/josm/command/ChangeNodesCommand.java (revision 15013) @@ -4,17 +4,10 @@ import static org.openstreetmap.josm.tools.I18n.tr; -import java.util.Collection; import java.util.List; -import java.util.Objects; - -import javax.swing.Icon; import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.data.osm.DefaultNameFormatter; import org.openstreetmap.josm.data.osm.Node; -import org.openstreetmap.josm.data.osm.OsmPrimitive; -import org.openstreetmap.josm.data.osm.OsmPrimitiveType; import org.openstreetmap.josm.data.osm.Way; -import org.openstreetmap.josm.tools.ImageProvider; /** @@ -22,12 +15,9 @@ * The same can be done with ChangeCommand, but this is more * efficient. (Needed for the duplicate node fixing - * tool of the validator plugin, when processing large data sets.) + * tool of the validator, when processing large data sets.) * * @author Imi */ -public class ChangeNodesCommand extends Command { - - private final Way way; - private final List newNodes; +public class ChangeNodesCommand extends AbstractNodesCommand> { /** @@ -37,5 +27,5 @@ */ public ChangeNodesCommand(Way way, List newNodes) { - this(way.getDataSet(), way, newNodes); + super(way.getDataSet(), way, newNodes); } @@ -48,23 +38,10 @@ */ public ChangeNodesCommand(DataSet ds, Way way, List newNodes) { - super(ds); - this.way = way; - this.newNodes = newNodes; - if (newNodes.isEmpty()) { - throw new IllegalArgumentException("Cannot set nodes to be an empty list."); - } + super(ds, way, newNodes); } @Override - public boolean executeCommand() { - super.executeCommand(); - way.setNodes(newNodes); - way.setModified(true); - return true; - } - - @Override - public void fillModifiedData(Collection modified, Collection deleted, Collection added) { - modified.add(way); + public void modifyWay() { + way.setNodes(cmdNodes); } @@ -73,23 +50,3 @@ return tr("Change nodes of {0}", way.getDisplayName(DefaultNameFormatter.getInstance())); } - - @Override - public Icon getDescriptionIcon() { - return ImageProvider.get(OsmPrimitiveType.WAY); - } - - @Override - public int hashCode() { - return Objects.hash(super.hashCode(), way, newNodes); - } - - @Override - public boolean equals(Object obj) { - if (this == obj) return true; - if (obj == null || getClass() != obj.getClass()) return false; - if (!super.equals(obj)) return false; - ChangeNodesCommand that = (ChangeNodesCommand) obj; - return Objects.equals(way, that.way) && - Objects.equals(newNodes, that.newNodes); - } } Index: trunk/src/org/openstreetmap/josm/command/RemoveNodesCommand.java =================================================================== --- trunk/src/org/openstreetmap/josm/command/RemoveNodesCommand.java (revision 15012) +++ trunk/src/org/openstreetmap/josm/command/RemoveNodesCommand.java (revision 15013) @@ -4,18 +4,12 @@ import static org.openstreetmap.josm.tools.I18n.tr; -import java.util.Collection; import java.util.HashSet; import java.util.List; -import java.util.Objects; import java.util.Set; -import javax.swing.Icon; - +import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.data.osm.DefaultNameFormatter; import org.openstreetmap.josm.data.osm.Node; -import org.openstreetmap.josm.data.osm.OsmPrimitive; -import org.openstreetmap.josm.data.osm.OsmPrimitiveType; import org.openstreetmap.josm.data.osm.Way; -import org.openstreetmap.josm.tools.ImageProvider; /** @@ -26,8 +20,5 @@ * @author Giuseppe Bilotta */ -public class RemoveNodesCommand extends Command { - - private final Way way; - private final Set rmNodes; +public class RemoveNodesCommand extends AbstractNodesCommand> { /** @@ -35,22 +26,35 @@ * @param way The way to modify. Must not be null, and belong to a data set * @param rmNodes The list of nodes to remove + * @deprecated Use {@link #RemoveNodesCommand(Way, Set)} */ + @Deprecated public RemoveNodesCommand(Way way, List rmNodes) { - super(way.getDataSet()); - this.way = way; - this.rmNodes = new HashSet<>(rmNodes); + super(way.getDataSet(), way, new HashSet<>(rmNodes)); + } + + /** + * Constructs a new {@code RemoveNodesCommand}. + * @param way The way to modify. Must not be null, and belong to a data set + * @param rmNodes The set of nodes to remove + * @since xxx + */ + public RemoveNodesCommand(Way way, Set rmNodes) { + super(way.getDataSet(), way, rmNodes); + } + + /** + * Constructs a new {@code RemoveNodesCommand}. + * @param ds The target data set. Must not be {@code null} + * @param way The way to modify. Must not be null, and belong to a data set + * @param rmNodes The list of nodes to remove + * @since 15013 + */ + public RemoveNodesCommand(DataSet ds, Way way, Set rmNodes) { + super(ds, way, rmNodes); } @Override - public boolean executeCommand() { - super.executeCommand(); - way.removeNodes(rmNodes); - way.setModified(true); - return true; - } - - @Override - public void fillModifiedData(Collection modified, Collection deleted, Collection added) { - modified.add(way); + protected void modifyWay() { + way.removeNodes(cmdNodes); } @@ -59,23 +63,3 @@ return tr("Removed nodes from {0}", way.getDisplayName(DefaultNameFormatter.getInstance())); } - - @Override - public Icon getDescriptionIcon() { - return ImageProvider.get(OsmPrimitiveType.WAY); - } - - @Override - public int hashCode() { - return Objects.hash(super.hashCode(), way, rmNodes); - } - - @Override - public boolean equals(Object obj) { - if (this == obj) return true; - if (obj == null || getClass() != obj.getClass()) return false; - if (!super.equals(obj)) return false; - RemoveNodesCommand that = (RemoveNodesCommand) obj; - return Objects.equals(way, that.way) && - Objects.equals(rmNodes, that.rmNodes); - } } Index: trunk/test/unit/org/openstreetmap/josm/command/RemoveNodesCommandTest.java =================================================================== --- trunk/test/unit/org/openstreetmap/josm/command/RemoveNodesCommandTest.java (revision 15012) +++ trunk/test/unit/org/openstreetmap/josm/command/RemoveNodesCommandTest.java (revision 15013) @@ -52,5 +52,5 @@ public void testExecute() { RemoveNodesCommand command = new RemoveNodesCommand(testData.existingWay, - Collections.singletonList(testData.existingNode)); + Collections.singleton(testData.existingNode)); command.executeCommand(); @@ -67,5 +67,5 @@ public void testUndo() { RemoveNodesCommand command = new RemoveNodesCommand(testData.existingWay, - Collections.singletonList(testData.existingNode)); + Collections.singleton(testData.existingNode)); command.executeCommand(); @@ -92,5 +92,5 @@ ArrayList added = new ArrayList<>(); RemoveNodesCommand command = new RemoveNodesCommand(testData.existingWay, - Collections.singletonList(testData.existingNode)); + Collections.singleton(testData.existingNode)); command.fillModifiedData(modified, deleted, added); assertArrayEquals(new Object[] {testData.existingWay }, modified.toArray()); @@ -105,5 +105,5 @@ public void testGetParticipatingPrimitives() { RemoveNodesCommand command = new RemoveNodesCommand(testData.existingWay, - Collections.singletonList(testData.existingNode)); + Collections.singleton(testData.existingNode)); command.executeCommand(); assertArrayEquals(new Object[] {testData.existingWay }, command.getParticipatingPrimitives().toArray()); @@ -115,5 +115,5 @@ @Test public void testDescription() { - assertTrue(new RemoveNodesCommand(testData.existingWay, Collections.singletonList(testData.existingNode)) + assertTrue(new RemoveNodesCommand(testData.existingWay, Collections.singleton(testData.existingNode)) .getDescriptionText().matches("Removed nodes from .*")); } Index: trunk/test/unit/org/openstreetmap/josm/gui/io/UploadDialogTest.java =================================================================== --- trunk/test/unit/org/openstreetmap/josm/gui/io/UploadDialogTest.java (revision 15012) +++ trunk/test/unit/org/openstreetmap/josm/gui/io/UploadDialogTest.java (revision 15013) @@ -280,5 +280,5 @@ */ @Test - public void testvalidateUploadTag() { + public void testValidateUploadTag() { doTestValidateUploadTag("upload.comment"); doTestValidateUploadTag("upload.source");