Changeset 10299 in josm for trunk

Timestamp:

2016-05-29T16:33:08+02:00 (8 years ago)

Author:

Don-vip

Message:

sonar - Performance - Method passes constant String of length 1 to character overridden method + add unit tests/javadoc

Location:

trunk

Files:

• src/org/openstreetmap/josm/actions/ExtensionFileFilter.java (modified) (1 diff)
• src/org/openstreetmap/josm/actions/search/SearchCompiler.java (modified) (9 diffs)
• src/org/openstreetmap/josm/data/AutosaveTask.java (modified) (8 diffs)
• test/unit/org/openstreetmap/josm/actions/ExtensionFileFilterTest.java (modified) (2 diffs)
• test/unit/org/openstreetmap/josm/actions/search/SearchCompilerTest.java (modified) (1 diff)
• test/unit/org/openstreetmap/josm/data/AutosaveTaskTest.java (added)

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

@@ -319 +319 @@
             defaultExtension,
             description + (!extensionsForDescription.isEmpty()
-                ? (" (" + Utils.join(", ", extensionsForDescription) + ")")
+                ? (" (" + Utils.join(", ", extensionsForDescription) + ')')
                 : "")
             );

trunk/src/org/openstreetmap/josm/actions/search/SearchCompiler.java

@@ -244 +244 @@
 
     public interface BinaryMatchFactory extends MatchFactory {
-        BinaryMatch get(String keyword, Match lhs, Match rhs, PushbackTokenizer tokenizer) throws ParseError;
+        AbstractBinaryMatch get(String keyword, Match lhs, Match rhs, PushbackTokenizer tokenizer) throws ParseError;
     }
 
@@ -337 +337 @@
      * A binary search operator which may take data parameters.
      */
-    public abstract static class BinaryMatch extends Match {
+    public abstract static class AbstractBinaryMatch extends Match {
 
         protected final Match lhs;
         protected final Match rhs;
 
-        public BinaryMatch(Match lhs, Match rhs) {
+        /**
+         * Constructs a new {@code BinaryMatch}.
+         * @param lhs Left hand side
+         * @param rhs Right hand side
+         */
+        public AbstractBinaryMatch(Match lhs, Match rhs) {
             this.lhs = lhs;
             this.rhs = rhs;
         }
 
-        public Match getLhs() {
+        /**
+         * Returns left hand side.
+         * @return left hand side
+         */
+        public final Match getLhs() {
             return lhs;
         }
 
-        public Match getRhs() {
+        /**
+         * Returns right hand side.
+         * @return right hand side
+         */
+        public final Match getRhs() {
             return rhs;
         }
@@ -438 +451 @@
      * Matches if both left and right expressions match.
      */
-    public static class And extends BinaryMatch {
+    public static class And extends AbstractBinaryMatch {
+        /**
+         * Constructs a new {@code And} match.
+         * @param lhs left hand side
+         * @param rhs right hand side
+         */
         public And(Match lhs, Match rhs) {
             super(lhs, rhs);
@@ -455 +473 @@
         @Override
         public String toString() {
-            return (lhs instanceof BinaryMatch && !(lhs instanceof And) ? "(" + lhs + ")" : lhs) + " && "
-                    + (rhs instanceof BinaryMatch && !(rhs instanceof And) ? "(" + rhs + ")" : rhs);
+            return (lhs instanceof AbstractBinaryMatch && !(lhs instanceof And) ? ("(" + lhs + ')') : lhs) + " && "
+                    + (rhs instanceof AbstractBinaryMatch && !(rhs instanceof And) ? ("(" + rhs + ')') : rhs);
         }
     }
@@ -463 +481 @@
      * Matches if the left OR the right expression match.
      */
-    public static class Or extends BinaryMatch {
+    public static class Or extends AbstractBinaryMatch {
+        /**
+         * Constructs a new {@code Or} match.
+         * @param lhs left hand side
+         * @param rhs right hand side
+         */
         public Or(Match lhs, Match rhs) {
             super(lhs, rhs);
@@ -480 +503 @@
         @Override
         public String toString() {
-            return (lhs instanceof BinaryMatch && !(lhs instanceof Or) ? "(" + lhs + ")" : lhs) + " || "
-                    + (rhs instanceof BinaryMatch && !(rhs instanceof Or) ? "(" + rhs + ")" : rhs);
+            return (lhs instanceof AbstractBinaryMatch && !(lhs instanceof Or) ? ("(" + lhs + ')') : lhs) + " || "
+                    + (rhs instanceof AbstractBinaryMatch && !(rhs instanceof Or) ? ("(" + rhs + ')') : rhs);
         }
     }
@@ -488 +511 @@
      * Matches if the left OR the right expression match, but not both.
      */
-    public static class Xor extends BinaryMatch {
+    public static class Xor extends AbstractBinaryMatch {
+        /**
+         * Constructs a new {@code Xor} match.
+         * @param lhs left hand side
+         * @param rhs right hand side
+         */
         public Xor(Match lhs, Match rhs) {
             super(lhs, rhs);
@@ -505 +533 @@
         @Override
         public String toString() {
-            return (lhs instanceof BinaryMatch && !(lhs instanceof Xor) ? "(" + lhs + ")" : lhs) + " ^ "
-                    + (rhs instanceof BinaryMatch && !(rhs instanceof Xor) ? "(" + rhs + ")" : rhs);
+            return (lhs instanceof AbstractBinaryMatch && !(lhs instanceof Xor) ? ("(" + lhs + ')') : lhs) + " ^ "
+                    + (rhs instanceof AbstractBinaryMatch && !(rhs instanceof Xor) ? ("(" + rhs + ')') : rhs);
         }
     }
@@ -1755 +1783 @@
         final String forKey = '"' + escapeStringForSearch(key) + '"' + '=';
         if (value == null || value.isEmpty()) {
-            return forKey + "*";
+            return forKey + '*';
         } else {
             return forKey + '"' + escapeStringForSearch(value) + '"';

trunk/src/org/openstreetmap/josm/data/AutosaveTask.java

@@ -13 +13 @@
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Date;
@@ -73 +74 @@
     public static final BooleanProperty PROP_NOTIFICATION = new BooleanProperty("autosave.notification", false);
 
-    private static class AutosaveLayerInfo {
-        private OsmDataLayer layer;
+    protected static final class AutosaveLayerInfo {
+        private final OsmDataLayer layer;
         private String layerName;
         private String layerFileName;
         private final Deque<File> backupFiles = new LinkedList<>();
+
+        AutosaveLayerInfo(OsmDataLayer layer) {
+            this.layer = layer;
+        }
     }
 
@@ -89 +94 @@
     private final File autosaveDir = new File(Main.pref.getUserDataDirectory(), AUTOSAVE_DIR);
     private final File deletedLayersDir = new File(Main.pref.getUserDataDirectory(), DELETED_LAYERS_DIR);
+
+    /**
+     * Replies the autosave directory.
+     * @return the autosave directory
+     * @since 10299
+     */
+    public final Path getAutosaveDir() {
+        return autosaveDir.toPath();
+    }
 
     public void schedule() {
@@ -153 +167 @@
     }
 
-    private File getNewLayerFile(AutosaveLayerInfo layer) {
-        int index = 0;
-        Date now = new Date();
+    protected File getNewLayerFile(AutosaveLayerInfo layer, Date now, int startIndex) {
+        int index = startIndex;
         while (true) {
             String filename = String.format("%1$s_%2$tY%2$tm%2$td_%2$tH%2$tM%2$tS%2$tL%3$s",
-                    layer.layerFileName, now, index == 0 ? "" : '_' + index);
-            File result = new File(autosaveDir, filename + "." + Main.pref.get("autosave.extension", "osm"));
+                    layer.layerFileName, now, index == 0 ? "" : ("_" + index));
+            File result = new File(autosaveDir, filename + '.' + Main.pref.get("autosave.extension", "osm"));
             try {
+                if (index > PROP_INDEX_LIMIT.get())
+                    throw new IOException("index limit exceeded");
                 if (result.createNewFile()) {
-                    File pidFile = new File(autosaveDir, filename+".pid");
-                    try (PrintStream ps = new PrintStream(pidFile, "UTF-8")) {
-                        ps.println(ManagementFactory.getRuntimeMXBean().getName());
-                    } catch (IOException | SecurityException t) {
-                        Main.error(t);
-                    }
+                    createNewPidFile(autosaveDir, filename);
                     return result;
                 } else {
                     Main.warn(tr("Unable to create file {0}, other filename will be used", result.getAbsolutePath()));
-                    if (index > PROP_INDEX_LIMIT.get())
-                        throw new IOException("index limit exceeded");
                 }
             } catch (IOException e) {
@@ -179 +187 @@
             }
             index++;
+        }
+    }
+
+    private static void createNewPidFile(File autosaveDir, String filename) {
+        File pidFile = new File(autosaveDir, filename+".pid");
+        try (PrintStream ps = new PrintStream(pidFile, "UTF-8")) {
+            ps.println(ManagementFactory.getRuntimeMXBean().getName());
+        } catch (IOException | SecurityException t) {
+            Main.error(t);
         }
     }
@@ -188 +205 @@
         }
         if (changedDatasets.remove(info.layer.data)) {
-            File file = getNewLayerFile(info);
+            File file = getNewLayerFile(info, new Date(), 0);
             if (file != null) {
                 info.backupFiles.add(file);
@@ -240 +257 @@
         synchronized (layersLock) {
             layer.data.addDataSetListener(datasetAdapter);
-            AutosaveLayerInfo info = new AutosaveLayerInfo();
-            info.layer = layer;
-            layersInfo.add(info);
+            layersInfo.add(new AutosaveLayerInfo(layer));
         }
     }
@@ -287 +302 @@
     }
 
-    private File getPidFile(File osmFile) {
+    protected File getPidFile(File osmFile) {
         return new File(autosaveDir, osmFile.getName().replaceFirst("[.][^.]+$", ".pid"));
     }

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

@@ -2 +2 @@
 package org.openstreetmap.josm.actions;
 
-import nl.jqno.equalsverifier.EqualsVerifier;
+import static org.junit.Assert.assertEquals;
 
 import org.junit.Test;
+
+import nl.jqno.equalsverifier.EqualsVerifier;
 
 /**
@@ -10 +12 @@
  */
 public class ExtensionFileFilterTest {
+
+    private static void test(String extensions, String defaultExtension, String description, boolean addArchiveExtensionsToDescription,
+            String expectedExtensions, String expectedDescription) {
+        ExtensionFileFilter ext = ExtensionFileFilter.newFilterWithArchiveExtensions(
+                extensions, defaultExtension, description, addArchiveExtensionsToDescription);
+        assertEquals(expectedExtensions, ext.getExtensions());
+        assertEquals(defaultExtension, ext.getDefaultExtension());
+        assertEquals(expectedDescription, ext.getDescription());
+    }
+
+    /**
+     * Unit test of method {@link ExtensionFileFilter#newFilterWithArchiveExtensions}.
+     */
+    @Test
+    public void testNewFilterWithArchiveExtensions() {
+        test("ext1", "ext1", "description", true,
+                "ext1,ext1.gz,ext1.bz2", "description (*.ext1, *.ext1.gz, *.ext1.bz2)");
+        test("ext1", "ext1", "description", false,
+                "ext1,ext1.gz,ext1.bz2", "description (*.ext1)");
+        test("ext1,ext2", "ext1", "description", true,
+                "ext1,ext1.gz,ext1.bz2,ext2,ext2.gz,ext2.bz2", "description (*.ext1, *.ext1.gz, *.ext1.bz2, *.ext2, *.ext2.gz, *.ext2.bz2)");
+        test("ext1,ext2", "ext1", "description", false,
+                "ext1,ext1.gz,ext1.bz2,ext2,ext2.gz,ext2.bz2", "description (*.ext1, *.ext2)");
+    }
 
     /**

trunk/test/unit/org/openstreetmap/josm/actions/search/SearchCompilerTest.java

@@ -427 +427 @@
         final SearchCompiler.Match c4 = SearchCompiler.compile("foo1 OR (bar1 bar2 baz1 XOR baz2) OR foo2");
         assertEquals("foo1 || (bar1 && bar2 && (baz1 ^ baz2)) || foo2", c4.toString());
+        final SearchCompiler.Match c5 = SearchCompiler.compile("foo1 XOR (baz1 XOR (bar baz))");
+        assertEquals("foo1 ^ baz1 ^ (bar && baz)", c5.toString());
+        final SearchCompiler.Match c6 = SearchCompiler.compile("foo1 XOR ((baz1 baz2) XOR (bar OR baz))");
+        assertEquals("foo1 ^ (baz1 && baz2) ^ (bar || baz)", c6.toString());
     }