Changeset 11109 in josm

Timestamp:

2016-10-09T04:03:32+02:00 (8 years ago)

Author:

Don-vip

Message:

sonar - squid:S1871 - Two branches in the same conditional structure should not have exactly the same implementation

Location:

trunk

Files:

• src/org/openstreetmap/josm/actions/MergeNodesAction.java (modified) (2 diffs)
• src/org/openstreetmap/josm/actions/SessionSaveAsAction.java (modified) (1 diff)
• src/org/openstreetmap/josm/actions/SimplifyWayAction.java (modified) (1 diff)
• src/org/openstreetmap/josm/actions/UploadSelectionAction.java (modified) (1 diff)
• test/unit/org/openstreetmap/josm/actions/MergeNodesActionTest.java (modified) (2 diffs)
• test/unit/org/openstreetmap/josm/actions/SessionSaveAsActionTest.java (added)

trunk/src/org/openstreetmap/josm/actions/MergeNodesAction.java

@@ -161 +161 @@
                 // Among existing nodes, try to keep the oldest used one
                 if (!n.getReferrers().isEmpty()) {
-                    if (targetNode == null) {
-                        targetNode = n;
-                    } else if (n.getId() < targetNode.getId()) {
+                    if (targetNode == null || n.getId() < targetNode.getId()) {
                         targetNode = n;
                     }
-                } else if (oldestNode == null) {
-                    oldestNode = n;
-                } else if (n.getId() < oldestNode.getId()) {
+                } else if (oldestNode == null || n.getId() < oldestNode.getId()) {
                     oldestNode = n;
                 }
@@ -200 +196 @@
                 if (!nodesToDelete.contains(n) && !n.equals(targetNode)) {
                     newNodes.add(n);
-                } else if (newNodes.isEmpty()) {
-                    newNodes.add(targetNode);
-                } else if (!newNodes.get(newNodes.size()-1).equals(targetNode)) {
+                } else if (newNodes.isEmpty() || !newNodes.get(newNodes.size()-1).equals(targetNode)) {
                     // make sure we collapse a sequence of deleted nodes
                     // to exactly one occurrence of the merged target node

trunk/src/org/openstreetmap/josm/actions/SessionSaveAsAction.java

@@ -131 +131 @@
         boolean zip;
         FileFilter ff = fc.getFileFilter();
-        if (zipRequired) {
-            zip = true;
-        } else if (joz.equals(ff)) {
+        if (zipRequired || joz.equals(ff)) {
             zip = true;
         } else if (jos.equals(ff)) {

trunk/src/org/openstreetmap/josm/actions/SimplifyWayAction.java

@@ -101 +101 @@
                 alertSelectAtLeastOneWay();
                 return;
-            } else if (!confirmWayWithNodesOutsideBoundingBox(ways)) {
-                return;
-            } else if (ways.size() > 10 && !confirmSimplifyManyWays(ways.size())) {
+            } else if (!confirmWayWithNodesOutsideBoundingBox(ways) || (ways.size() > 10 && !confirmSimplifyManyWays(ways.size()))) {
                 return;
             }

trunk/src/org/openstreetmap/josm/actions/UploadSelectionAction.java

@@ -79 +79 @@
         Set<OsmPrimitive> ret = new HashSet<>();
         for (OsmPrimitive p: primitives) {
-            if (p.isNewOrUndeleted()) {
-                ret.add(p);
-            } else if (p.isModified() && !p.isIncomplete()) {
+            if (p.isNewOrUndeleted() || (p.isModified() && !p.isIncomplete())) {
                 ret.add(p);
             }

trunk/test/unit/org/openstreetmap/josm/actions/MergeNodesActionTest.java

@@ -3 +3 @@
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 
 import java.util.Arrays;
@@ -46 +47 @@
 
     /**
-     * Unit test of {@link MergeNodesAction#selectTargetLocationNode} - mode 0
+     * Unit test of {@link MergeNodesAction#selectTargetLocationNode}
      */
     @Test
-    public void testSelectTargetLocationNodeMode0() {
+    public void testSelectTargetLocationNode() {
         Main.pref.putInteger("merge-nodes.mode", 0);
         assertEquals(1, MergeNodesAction.selectTargetLocationNode(Arrays.asList(new Node(0), new Node(1))).getId());
-    }
 
-    /**
-     * Unit test of {@link MergeNodesAction#selectTargetLocationNode} - mode 1
-     */
-    @Test
-    public void testSelectTargetLocationNodeMode1() {
         Main.pref.putInteger("merge-nodes.mode", 1);
         assertEquals(LatLon.ZERO, MergeNodesAction.selectTargetLocationNode(
                 Arrays.asList(new Node(LatLon.NORTH_POLE), new Node(LatLon.SOUTH_POLE))).getCoor());
-    }
 
-    /**
-     * Unit test of {@link MergeNodesAction#selectTargetLocationNode} - mode 2 with a single node
-     */
-    @Test
-    public void testSelectTargetLocationNodeMode2SingleNode() {
         Main.pref.putInteger("merge-nodes.mode", 2);
         assertEquals(LatLon.NORTH_POLE, MergeNodesAction.selectTargetLocationNode(
                 Arrays.asList(new Node(LatLon.NORTH_POLE))).getCoor());
     }
+
+    /**
+     * Unit test of {@link MergeNodesAction#selectTargetNode}
+     */
+    @Test
+    public void testSelectTargetNode() {
+        assertNull(MergeNodesAction.selectTargetNode(Collections.emptyList()));
+        assertEquals(1, MergeNodesAction.selectTargetNode(Arrays.asList(new Node(1))).getId());
+    }
 }