gnucash maint: Improve transaction sorting on effective num field.

John Ralls jralls at code.gnucash.org
Tue Jun 15 17:03:06 EDT 2021


Updated	 via  https://github.com/Gnucash/gnucash/commit/601eb513 (commit)
	from  https://github.com/Gnucash/gnucash/commit/84fb25a0 (commit)



commit 601eb513619a9d8d9a609ac22d5e3b20527ed6ad
Author: John Ralls <jralls at ceridwen.us>
Date:   Tue Jun 8 09:49:46 2021 -0700

    Improve transaction sorting on effective num field.
    
    Inspired by PR #983. Transaction sorting on num broke down if the
    user had a non-numeric string or a number larger than an int in
    the effective num field (might be split-action if the option is set).
    
    The comparison first tries to use strtoull on the two strings and
    compares the results. If they're both nonzero and different then the
    numeric order is returned. If they're both nonzero but the same the
    unconverted parts of each are passed to g_utf8_collate; if either is 0
    then the whole strings are passed to g_utf8_collate. strtoull will
    return 0 for a negative number.

diff --git a/libgnucash/engine/Transaction.c b/libgnucash/engine/Transaction.c
index 2865b237e..e7c3cba0a 100644
--- a/libgnucash/engine/Transaction.c
+++ b/libgnucash/engine/Transaction.c
@@ -1881,12 +1881,43 @@ xaccTransOrder (const Transaction *ta, const Transaction *tb)
     return xaccTransOrder_num_action (ta, NULL, tb, NULL);
 }
 
+/* Order a pair of potentially numeric string as numbers if both
+ * strings begin with numbers, ordering the remainder of the string
+ * lexically if the numeric parts are equal, and the whole strings
+ * lexically otherwise.
+ *
+ * Note that this won't work well for numbers > 10^18 and that
+ * negative numbers are treated as strings and will cause the pair to
+ * be ordered lexically.
+ */
+
+static int
+order_by_int64_or_string (const char* a, const char* b)
+{
+     char *end_a = NULL, *end_b = NULL;
+     int cmp = 0;
+     uint64_t na = strtoull(a, &end_a, 10);
+     uint64_t nb = strtoull(b, &end_b, 10);
+     if (na && nb)
+     {
+          if (na != nb)
+               return na < nb ? -1 : 1;
+          cmp = g_utf8_collate(end_a, end_b);
+     }
+     else
+     {
+          cmp = g_utf8_collate(a, b);
+     }
+     return cmp < 0 ? -1 : cmp > 0 ? 1 : 0;
+}
+
 int
 xaccTransOrder_num_action (const Transaction *ta, const char *actna,
                             const Transaction *tb, const char *actnb)
 {
     char *da, *db;
-    int na, nb, retval;
+    int retval;
+    int64_t na, nb;
 
     if ( ta && !tb ) return -1;
     if ( !ta && tb ) return +1;
@@ -1906,16 +1937,14 @@ xaccTransOrder_num_action (const Transaction *ta, const char *actna,
     /* otherwise, sort on number string */
     if (actna && actnb) /* split action string, if not NULL */
     {
-        na = atoi(actna);
-        nb = atoi(actnb);
+         retval = order_by_int64_or_string (actna, actnb);
     }
     else                /* else transaction num string */
     {
-        na = atoi(ta->num);
-        nb = atoi(tb->num);
+         retval = order_by_int64_or_string (ta->num, tb->num);
     }
-    if (na < nb) return -1;
-    if (na > nb) return +1;
+    if (retval)
+         return retval;
 
     if (ta->date_entered != tb->date_entered)
         return (ta->date_entered > tb->date_entered) - (ta->date_entered < tb->date_entered);
diff --git a/libgnucash/engine/test/utest-Transaction.cpp b/libgnucash/engine/test/utest-Transaction.cpp
index ec8de4c4b..3cf10ebb8 100644
--- a/libgnucash/engine/test/utest-Transaction.cpp
+++ b/libgnucash/engine/test/utest-Transaction.cpp
@@ -1763,8 +1763,14 @@ test_xaccTransOrder_num_action (Fixture *fixture, gconstpointer pData)
     g_assert_cmpint (xaccTransOrder_num_action (txnA, NULL, txnB, NULL), ==, -1);
     txnB->num = static_cast<char*>(CACHE_INSERT ("101"));
     g_assert_cmpint (xaccTransOrder_num_action (txnA, NULL, txnB, NULL), ==, 1);
-    txnB->num = static_cast<char*>(CACHE_INSERT ("one-oh-one"));
+    txnA->num = static_cast<char*>(CACHE_INSERT ("12a"));
+    g_assert_cmpint (xaccTransOrder_num_action (txnA, NULL, txnB, NULL), ==, -1);
+    txnB->num = static_cast<char*>(CACHE_INSERT ("12c"));
+    g_assert_cmpint (xaccTransOrder_num_action (txnA, NULL, txnB, NULL), ==, -1);
+    txnB->num = static_cast<char*>(CACHE_INSERT ("12"));
     g_assert_cmpint (xaccTransOrder_num_action (txnA, NULL, txnB, NULL), ==, 1);
+    txnB->num = static_cast<char*>(CACHE_INSERT ("one-oh-one"));
+    g_assert_cmpint (xaccTransOrder_num_action (txnA, NULL, txnB, NULL), ==, -1);
     g_assert_cmpint (xaccTransOrder_num_action (txnA, "24", txnB, "42"), ==, -1);
     txnB->date_posted -= 1;
     g_assert_cmpint (xaccTransOrder_num_action (txnA, "24", txnB, "42"), ==, 1);



Summary of changes:
 libgnucash/engine/Transaction.c              | 43 +++++++++++++++++++++++-----
 libgnucash/engine/test/utest-Transaction.cpp |  8 +++++-
 2 files changed, 43 insertions(+), 8 deletions(-)



More information about the gnucash-changes mailing list