#6057 Improve startup time (#7486)

* check with regex instead of throwing exception * fix NumericFieldComparatorTest * change number validation method * add changes in CHANGELOG.md * fix fails on empty string * add case when only '-' * add NumericFieldComparator for number column only * add case for '+' sign * fix checkstyle * add corner case with '+' * add corner case with '+'
JabRef · Mar 10, 2021 · 50ba021 · 50ba021
1 parent c7593c9
commit 50ba021
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 32 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -97,6 +97,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 - We changed the title of the window  "Manage field names and content":  to have the same title as the corresponding menu item  [#6895](https://github.com/JabRef/jabref/pull/6895)
 - We improved the detection of "short" DOIs [6880](https://github.com/JabRef/jabref/issues/6880)
 - We improved the duplicate detection when identifiers like DOI or arxiv are semantiaclly the same, but just syntactically differ (e.g. with or without http(s):// prefix). [#6707](https://github.com/JabRef/jabref/issues/6707)
+- We improved JabRef start up time [6057](https://github.com/JabRef/jabref/issues/6057)
 - We changed in the group interface "Generate groups from keywords in a BibTeX field" by "Generate groups from keywords in the following field". [#6983](https://github.com/JabRef/jabref/issues/6983)
 - We changed the name of a group type from "Searching for keywords" to "Searching for a keyword". [6995](https://github.com/JabRef/jabref/pull/6995)
 - We changed the way JabRef displays the title of a tab and of the window. [4161](https://github.com/JabRef/jabref/issues/4161)

diff --git a/src/main/java/org/jabref/gui/maintable/columns/FieldColumn.java b/src/main/java/org/jabref/gui/maintable/columns/FieldColumn.java
@@ -6,8 +6,12 @@
 import org.jabref.gui.maintable.MainTableColumnModel;
 import org.jabref.gui.util.ValueTableCellFactory;
 import org.jabref.gui.util.comparator.NumericFieldComparator;
+import org.jabref.model.entry.field.Field;
 import org.jabref.model.entry.field.FieldFactory;
 import org.jabref.model.entry.field.OrFields;
+import org.jabref.model.entry.field.UnknownField;
+
+import com.google.common.collect.Iterables;
 
 /**
  * A column that displays the text-value of the field
@@ -26,7 +30,16 @@ public FieldColumn(MainTableColumnModel model) {
         new ValueTableCellFactory<BibEntryTableViewModel, String>()
                 .withText(text -> text)
                 .install(this);
-        this.setComparator(new NumericFieldComparator());
+
+        if (fields.size() == 1) {
+            // comparator can't parse more than one value
+            Field field = Iterables.getOnlyElement(fields);
+
+            if (field instanceof UnknownField || field.isNumeric()) {
+                this.setComparator(new NumericFieldComparator());
+            }
+        }
+
         this.setSortable(true);
     }
 

diff --git a/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java b/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java
@@ -2,6 +2,8 @@
 
 import java.util.Comparator;
 
+import org.jabref.model.strings.StringUtil;
+
 /**
  * Comparator for numeric cases. The purpose of this class is to add the numeric comparison, because values are sorted
  * as if they were strings.
@@ -10,51 +12,54 @@ public class NumericFieldComparator implements Comparator<String> {
 
     @Override
     public int compare(String val1, String val2) {
-        // We start by implementing the comparison in the edge cases (if one of the values is null).
-        if (val1 == null && val2 == null) {
-            return 0;
-        }
+        Integer valInt1 = parseInt(val1);
+        Integer valInt2 = parseInt(val2);
 
-        if (val1 == null) {
+        if (valInt1 == null && valInt2 == null) {
+            if (val1 != null && val2 != null) {
+                return val1.compareTo(val2);
+            } else {
+                return 0;
+            }
+        } else if (valInt1 == null) {
             // We assume that "null" is "less than" any other value.
             return -1;
-        }
-
-        if (val2 == null) {
+        } else if (valInt2 == null) {
             return 1;
         }
 
-        // Now we start the conversion to integers.
-        Integer valInt1 = null;
-        Integer valInt2 = null;
-        try {
-            // Trim in case the user added an unnecessary white space (e.g. 1 1 instead of 11).
-            valInt1 = Integer.parseInt(val1.trim());
-        } catch (NumberFormatException ignore) {
-            // do nothing
+        // If we arrive at this stage then both values are actually numeric !
+        return valInt1 - valInt2;
+    }
+
+    private static Integer parseInt(String number) {
+        if (!isNumber(number)) {
+            return null;
         }
+
         try {
-            valInt2 = Integer.parseInt(val2.trim());
+            return Integer.valueOf(number.trim());
         } catch (NumberFormatException ignore) {
-            // do nothing
+            return null;
         }
+    }
 
-        if (valInt1 == null && valInt2 == null) {
-            // None of the values were parsed (i.e both are not numeric)
-            // so we will use the normal string comparison.
-            return val1.compareTo(val2);
+    private static boolean isNumber(String number) {
+        if (StringUtil.isNullOrEmpty(number)) {
+            return false;
         }
-
-        if (valInt1 == null) {
-            // We assume that strings "are less" than integers
-            return -1;
+        if (number.length() == 1 && (number.charAt(0) == '-' || number.charAt(0) == '+')) {
+            return false;
         }
-
-        if (valInt2 == null) {
-            return 1;
+        for (int i = 0; i < number.length(); i++) {
+            char c = number.charAt(i);
+            if (i == 0 && (c == '-' || c == '+')) {
+                continue;
+            } else if (!Character.isDigit(c)) {
+                return false;
+            }
         }
 
-        // If we arrive at this stage then both values are actually numeric !
-        return valInt1 - valInt2;
+        return true;
     }
 }
diff --git a/src/test/java/org/jabref/gui/util/comparator/NumericFieldComparatorTest.java b/src/test/java/org/jabref/gui/util/comparator/NumericFieldComparatorTest.java
@@ -42,4 +42,24 @@ public void compareStringWithInteger() {
     public void compareIntegerWithString() {
         assertEquals(1, comparator.compare("4", "hi"));
     }
+
+    @Test
+    public void compareNegativeInteger() {
+        assertEquals(1, comparator.compare("-4", "-5"));
+    }
+
+    @Test
+    public void compareWithMinusString() {
+        assertEquals(-1, comparator.compare("-", "-5"));
+    }
+
+    @Test
+    public void compareWithPlusString() {
+        assertEquals(-1, comparator.compare("+", "-5"));
+    }
+
+    @Test
+    public void compareWordWithMinus() {
+        assertEquals(-1, comparator.compare("-abc", "-5"));
+    }
 }