From 50ba021ccf53105559b29b4c9b808f35ad46d142 Mon Sep 17 00:00:00 2001 From: Ali Date: Wed, 10 Mar 2021 21:32:18 +0300 Subject: [PATCH] #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 '+' --- CHANGELOG.md | 1 + .../gui/maintable/columns/FieldColumn.java | 15 ++++- .../comparator/NumericFieldComparator.java | 67 ++++++++++--------- .../NumericFieldComparatorTest.java | 20 ++++++ 4 files changed, 71 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ff8359f425..a07c4e120d2 100644 --- 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 index 2822f5bce30..aee93952cd7 100644 --- 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() .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 index 79601439b9c..38dd0d535ab 100644 --- 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 { @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 index 12fcde6059c..be11abf0abe 100644 --- 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")); + } }