From 46f9fb01c972615ff77152f0c76489d761925e15 Mon Sep 17 00:00:00 2001
From: John Ralls <jralls@ceridwen.us>
Date: Tue, 24 Jan 2023 09:46:48 -0800
Subject: [PATCH] Simplify gnc_commodity equality and compare functions.

gnc_commodity_compare is used for sorting and so needs to have a less-than
return value. The only place it's used presents nothing to the UI; a GncGUID
ordering is fast and stable.

GnuCash allows only one commodity per book for any namespace/mnemonic pair, so
the exhaustive string comparisons are superflous. While the current engine
design allows only one instance of any object, meaning that a pointer comparison
is sufficient to determine equality, that may not be true in the future, but the
GncGUID is guaranteed to sufficiently identify a single commodity. Note that
gnc_commodity_equiv is used to enforce that single-commodity rule and so cannot
use GncGUID comparison.
---
 libgnucash/engine/gnc-commodity.c | 71 ++++++-------------------------
 1 file changed, 12 insertions(+), 59 deletions(-)

diff --git a/libgnucash/engine/gnc-commodity.c b/libgnucash/engine/gnc-commodity.c
index ac686c4b7b..4d5ecea61d 100644
--- a/libgnucash/engine/gnc-commodity.c
+++ b/libgnucash/engine/gnc-commodity.c
@@ -38,6 +38,8 @@
 #include "gnc-commodity.h"
 #include "gnc-locale-utils.h"
 #include "gnc-prefs.h"
+#include "guid.h"
+#include "qofinstance.h"
 
 static QofLogModule log_module = GNC_MOD_COMMODITY;
 
@@ -1647,77 +1649,28 @@ gnc_commodity_equiv(const gnc_commodity * a, const gnc_commodity * b)
     priv_b = GET_PRIVATE(b);
     if (priv_a->name_space != priv_b->name_space) return FALSE;
     if (g_strcmp0(priv_a->mnemonic, priv_b->mnemonic) != 0) return FALSE;
+
     return TRUE;
 }
 
 gboolean
 gnc_commodity_equal(const gnc_commodity * a, const gnc_commodity * b)
 {
-    gnc_commodityPrivate* priv_a;
-    gnc_commodityPrivate* priv_b;
-    gboolean same_book;
-
-    if (a == b) return TRUE;
-
-    if (!a || !b)
-    {
-        DEBUG ("one is NULL");
-        return FALSE;
-    }
-
-    priv_a = GET_PRIVATE(a);
-    priv_b = GET_PRIVATE(b);
-    same_book = qof_instance_get_book(QOF_INSTANCE(a)) == qof_instance_get_book(QOF_INSTANCE(b));
-
-    if ((same_book && priv_a->name_space != priv_b->name_space)
-            || (!same_book && g_strcmp0( gnc_commodity_namespace_get_name(priv_a->name_space),
-                                           gnc_commodity_namespace_get_name(priv_b->name_space)) != 0))
-    {
-        DEBUG ("namespaces differ: %p(%s) vs %p(%s)",
-               priv_a->name_space, gnc_commodity_namespace_get_name(priv_a->name_space),
-               priv_b->name_space, gnc_commodity_namespace_get_name(priv_b->name_space));
-        return FALSE;
-    }
-
-    if (g_strcmp0(priv_a->mnemonic, priv_b->mnemonic) != 0)
-    {
-        DEBUG ("mnemonics differ: %s vs %s", priv_a->mnemonic, priv_b->mnemonic);
-        return FALSE;
-    }
-
-    if (g_strcmp0(priv_a->fullname, priv_b->fullname) != 0)
-    {
-        DEBUG ("fullnames differ: %s vs %s", priv_a->fullname, priv_b->fullname);
-        return FALSE;
-    }
-
-    if (g_strcmp0(priv_a->cusip, priv_b->cusip) != 0)
-    {
-        DEBUG ("cusips differ: %s vs %s", priv_a->cusip, priv_b->cusip);
-        return FALSE;
-    }
-
-    if (priv_a->fraction != priv_b->fraction)
-    {
-        DEBUG ("fractions differ: %d vs %d", priv_a->fraction, priv_b->fraction);
-        return FALSE;
-    }
-
-    return TRUE;
+    return gnc_commodity_compare(a, b) == 0;
 }
 
+// Used as a sorting callback for deleting old prices, so it needs to be
+// stable but doesn't need to be in any particular order sensible to humans.
 int gnc_commodity_compare(const gnc_commodity * a, const gnc_commodity * b)
 {
-    if (gnc_commodity_equal(a, b))
-    {
-        return 0;
-    }
-    else
-    {
-        return 1;
-    }
+    if (a == b) return 0;
+    if (a && !b) return 1;
+    if (b && !a) return -1;
+    return qof_instance_guid_compare(a, b);
 }
 
+// Used as a callback to g_list_find_custom, it should return 0
+// when the commodities match.
 int gnc_commodity_compare_void(const void * a, const void * b)
 {
     return gnc_commodity_compare(a, b);