Merge pull request #6450 from JabRef/dimitra-6349

Dimitra 6349

CHANGELOG.md

@@ -55,6 +55,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 - We fixed an issue where JabRef switched to discrete graphics under macOS [#5935](https://github.com/JabRef/jabref/issues/5935)
 - We fixed an issue where the Preferences entry preview will be unexpected modified leads to Value too long exception [#6198](https://github.com/JabRef/jabref/issues/6198)
 - We fixed an issue where custom jstyles for Open/LibreOffice would only be valid if a layout line for the entry type `default` was at the end of the layout section [#6303](https://github.com/JabRef/jabref/issues/6303)
+- We fixed an issue where sort on numeric cases was broken. [#6349](https://github.com/JabRef/jabref/issues/6349)
 
 
 ### Removed

src/main/java/org/jabref/gui/maintable/MainTableColumnFactory.java

@@ -38,6 +38,7 @@
 import org.jabref.gui.specialfields.SpecialFieldsPreferences;
 import org.jabref.gui.util.OptionalValueTableCellFactory;
 import org.jabref.gui.util.ValueTableCellFactory;
+import org.jabref.gui.util.comparator.NumericFieldComparator;
 import org.jabref.gui.util.comparator.PriorityFieldComparator;
 import org.jabref.gui.util.comparator.RankingFieldComparator;
 import org.jabref.gui.util.comparator.ReadStatusFieldComparator;
@@ -212,6 +213,7 @@ private Node createGroupColorRegion(BibEntryTableViewModel entry, List<AbstractG
         new ValueTableCellFactory<BibEntryTableViewModel, String>()
                 .withText(text -> text)
                 .install(column);
+        column.setComparator(new NumericFieldComparator());
         column.setSortable(true);
         return column;
     }

src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java

@@ -0,0 +1,60 @@
+package org.jabref.gui.util.comparator;
+
+import java.util.Comparator;
+
+/**
+ * Comparator for numeric cases. The purpose of this class is to add the numeric comparison, because values are sorted
+ * as if they were strings.
+ */
+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;
+        }
+
+        if (val1 == null) {
+            // We assume that "null" is "less than" any other value.
+            return -1;
+        }
+
+        if (val2 == 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
+        }
+        try {
+            valInt2 = Integer.parseInt(val2.trim());
+        } catch (NumberFormatException ignore) {
+            // do nothing
+        }
+
+        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);
+        }
+
+        if (valInt1 == null) {
+            // We assume that strings "are less" than integers
+            return -1;
+        }
+
+        if (valInt2 == null) {
+            return 1;
+        }
+
+        // If we arrive at this stage then both values are actually numeric !
+        return valInt1 - valInt2;
+    }
+}

src/test/java/org/jabref/gui/util/comparator/NumericFieldComparatorTest.java

@@ -0,0 +1,45 @@
+package org.jabref.gui.util.comparator;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class NumericFieldComparatorTest {
+
+    private final NumericFieldComparator comparator = new NumericFieldComparator();
+
+    @Test
+    public void compareTwoNumericInputs() {
+        assertEquals(2, comparator.compare("4", "2"));
+    }
+
+    @Test
+    public void compareTwoNullInputs() {
+        assertEquals(0, comparator.compare(null, null));
+    }
+
+    @Test
+    public void compareTwoInputsWithFirstNull() {
+        assertEquals(-1, comparator.compare(null, "2"));
+    }
+
+    @Test
+    public void compareTwoInputsWithSecondNull() {
+        assertEquals(1, comparator.compare("4", null));
+    }
+
+    @Test
+    public void compareTwoNotNumericInputs() {
+        assertEquals(-32, comparator.compare("HELLO", "hello"));
+    }
+
+    @Test
+    public void compareStringWithInteger() {
+        assertEquals(-1, comparator.compare("hi", "2"));
+    }
+
+    @Test
+    public void compareIntegerWithString() {
+        assertEquals(1, comparator.compare("4", "hi"));
+    }
+}