Use C++ STL instead of GLib

libgnucash/engine/gnc-budget.cpp

@@ -30,6 +30,9 @@
 #include <qof.h>
 #include <qofbookslots.h>
 #include <qofinstance-p.h>
+#include <unordered_map>
+#include <vector>
+#include <memory>
 
 #include "Account.h"
 
@@ -59,6 +62,16 @@ typedef struct
     QofInstanceClass parent_class;
 } BudgetClass;
 
+struct PeriodData
+{
+    std::string note;
+    bool value_is_set;
+    gnc_numeric value;
+};
+
+using PeriodDataVec = std::vector<PeriodData>;
+using AcctMap = std::unordered_map<const Account*, PeriodDataVec>;
+
 typedef struct GncBudgetPrivate
 {
     /* The name is an arbitrary string assigned by the user. */
@@ -70,7 +83,7 @@ typedef struct GncBudgetPrivate
     /* Recurrence (period info) for the budget */
     Recurrence recurrence;
 
-    GHashTable *acct_hash;
+    std::unique_ptr<AcctMap> acct_map;
 
     /* Number of periods */
     guint  num_periods;
@@ -87,24 +100,6 @@ struct _GncBudgetClass
 /* GObject Initialization */
 G_DEFINE_TYPE_WITH_PRIVATE(GncBudget, gnc_budget, QOF_TYPE_INSTANCE)
 
-typedef struct PeriodData
-{
-    gchar *note;
-    gboolean value_is_set;
-    gnc_numeric value;
-} PeriodData;
-
-static void acct_array_free (GArray *acct_array, gpointer user_data)
-{
-    for (guint i = 0; i < acct_array->len; i++)
-    {
-        PeriodData *perioddata = &g_array_index (acct_array, PeriodData, i);
-        if (perioddata->note)
-            g_free (perioddata->note);
-    }
-    g_array_free (acct_array, FALSE);
-}
-
 static void
 gnc_budget_init(GncBudget* budget)
 {
@@ -114,9 +109,7 @@ gnc_budget_init(GncBudget* budget)
     priv = GET_PRIVATE(budget);
     priv->name = CACHE_INSERT(_("Unnamed Budget"));
     priv->description = CACHE_INSERT("");
-
+    priv->acct_map = std::make_unique<AcctMap>();
-    priv->acct_hash = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL,
-                                             (GDestroyNotify) acct_array_free);
 
     priv->num_periods = 12;
     date = gnc_g_date_new_today ();
@@ -286,8 +279,7 @@ gnc_budget_free(QofInstance *inst)
 
     CACHE_REMOVE(priv->name);
     CACHE_REMOVE(priv->description);
-
+    priv->acct_map = nullptr;   // nullify to ensure unique_ptr is freed.
-    g_hash_table_destroy (priv->acct_hash);
 
     /* qof_instance_release (&budget->inst); */
     g_object_unref(budget);
@@ -471,13 +463,6 @@ gnc_budget_get_guid(const GncBudget* budget)
     return qof_instance_get_guid(QOF_INSTANCE(budget));
 }
 
-static void
-set_num_periods (GArray *perioddata, gpointer user_data)
-{
-    gint num_periods = GPOINTER_TO_INT (user_data);
-    g_array_set_size (perioddata, num_periods);
-}
-
 void
 gnc_budget_set_num_periods(GncBudget* budget, guint num_periods)
 {
@@ -493,8 +478,12 @@ gnc_budget_set_num_periods(GncBudget* budget, guint num_periods)
     qof_instance_set_dirty(&budget->inst);
     gnc_budget_commit_edit(budget);
 
-    g_hash_table_foreach (priv->acct_hash, (GHFunc) set_num_periods,
+    std::for_each (priv->acct_map->begin(),
-                          GUINT_TO_POINTER (num_periods));
+                   priv->acct_map->end(),
+                   [num_periods](auto& it)
+                   {
+                       it.second.resize(num_periods);
+                   });
 
     qof_event_gen( &budget->inst, QOF_EVENT_MODIFY, NULL);
 }
@@ -516,8 +505,9 @@ make_period_path (const Account *account, guint period_num, char *path1, char *p
     g_sprintf (path2, "%d", period_num);
 }
 
-static GArray* get_acct_array (const GncBudget *budget, const Account *account);
+static PeriodData& get_perioddata (const GncBudget *budget,
-
+                                   const Account *account,
+                                   guint period_num);
 
 /* period_num is zero-based */
 /* What happens when account is deleted, after we have an entry for it? */
@@ -525,17 +515,15 @@ void
 gnc_budget_unset_account_period_value(GncBudget *budget, const Account *account,
                                       guint period_num)
 {
-    GArray *acct_array;
     gchar path_part_one [GUID_ENCODING_LENGTH + 1];
     gchar path_part_two [GNC_BUDGET_MAX_NUM_PERIODS_DIGITS];
-    PeriodData *perioddata;
 
     g_return_if_fail (budget != NULL);
     g_return_if_fail (account != NULL);
+    g_return_if_fail (period_num < GET_PRIVATE(budget)->num_periods);
 
-    acct_array = get_acct_array (budget, account);
+    auto& data = get_perioddata (budget, account, period_num);
-    perioddata = &g_array_index (acct_array, PeriodData, period_num);
+    data.value_is_set = false;
-    perioddata->value_is_set = FALSE;
 
     make_period_path (account, period_num, path_part_one, path_part_two);
 
@@ -556,8 +544,6 @@ gnc_budget_set_account_period_value(GncBudget *budget, const Account *account,
 {
     gchar path_part_one [GUID_ENCODING_LENGTH + 1];
     gchar path_part_two [GNC_BUDGET_MAX_NUM_PERIODS_DIGITS];
-    GArray *acct_array;
-    PeriodData *perioddata;
 
     /* Watch out for an off-by-one error here:
      * period_num starts from 0 while num_periods starts from 1 */
@@ -570,8 +556,7 @@ gnc_budget_set_account_period_value(GncBudget *budget, const Account *account,
     g_return_if_fail (budget != NULL);
     g_return_if_fail (account != NULL);
 
-    acct_array = get_acct_array (budget, account);
+    auto& perioddata = get_perioddata (budget, account, period_num);
-    perioddata = &g_array_index (acct_array, PeriodData, period_num);
 
     make_period_path (account, period_num, path_part_one, path_part_two);
 
@@ -579,7 +564,7 @@ gnc_budget_set_account_period_value(GncBudget *budget, const Account *account,
     if (gnc_numeric_check(val))
     {
         qof_instance_set_kvp (QOF_INSTANCE (budget), NULL, 2, path_part_one, path_part_two);
-        perioddata->value_is_set = FALSE;
+        perioddata.value_is_set = false;
     }
     else
     {
@@ -588,8 +573,8 @@ gnc_budget_set_account_period_value(GncBudget *budget, const Account *account,
         g_value_set_boxed (&v, &val);
         qof_instance_set_kvp (QOF_INSTANCE (budget), &v, 2, path_part_one, path_part_two);
         g_value_unset (&v);
-        perioddata->value_is_set = TRUE;
+        perioddata.value_is_set = true;
-        perioddata->value = val;
+        perioddata.value = val;
     }
     qof_instance_set_dirty(&budget->inst);
     gnc_budget_commit_edit(budget);
@@ -627,9 +612,8 @@ gnc_budget_is_account_period_value_set (const GncBudget *budget,
                                         const Account *account,
                                         guint period_num)
 {
-    GArray *array = get_acct_array (budget, account);
+    g_return_val_if_fail (period_num < GET_PRIVATE(budget)->num_periods, false);
-    PeriodData *data = (PeriodData*) &g_array_index (array, PeriodData, period_num);
+    return get_perioddata (budget, account, period_num).value_is_set;
-    return data->value_is_set;
 }
 
 static gnc_numeric
@@ -661,9 +645,13 @@ gnc_budget_get_account_period_value (const GncBudget *budget,
                                      const Account *account,
                                      guint period_num)
 {
-    GArray *array = get_acct_array (budget, account);
+    g_return_val_if_fail (period_num < GET_PRIVATE(budget)->num_periods,
-    PeriodData *data = (PeriodData*) &g_array_index (array, PeriodData, period_num);
+                          gnc_numeric_zero());
-    return data->value_is_set ? data->value : gnc_numeric_zero ();
+    auto& data = get_perioddata (budget, account, period_num);
+    if (!data.value_is_set)
+        return gnc_numeric_zero();
+
+    return data.value;
 }
 
 void
@@ -672,8 +660,6 @@ gnc_budget_set_account_period_note(GncBudget *budget, const Account *account,
 {
     gchar path_part_one [GUID_ENCODING_LENGTH + 1];
     gchar path_part_two [GNC_BUDGET_MAX_NUM_PERIODS_DIGITS];
-    GArray *acct_array;
-    PeriodData *perioddata;
 
     /* Watch out for an off-by-one error here:
      * period_num starts from 0 while num_periods starts from 1 */
@@ -686,17 +672,16 @@ gnc_budget_set_account_period_note(GncBudget *budget, const Account *account,
     g_return_if_fail (budget != NULL);
     g_return_if_fail (account != NULL);
 
-    acct_array = get_acct_array (budget, account);
+    auto& perioddata = get_perioddata (budget, account, period_num);
-    perioddata = &g_array_index (acct_array, PeriodData, period_num);
 
     make_period_path (account, period_num, path_part_one, path_part_two);
 
     gnc_budget_begin_edit(budget);
-    if (perioddata->note)
-        g_free (perioddata->note);
-    perioddata->note = g_strdup (note);
     if (note == NULL)
+    {
         qof_instance_set_kvp (QOF_INSTANCE (budget), NULL, 3, GNC_BUDGET_NOTES_PATH, path_part_one, path_part_two);
+        perioddata.note.clear ();
+    }
     else
     {
         GValue v = G_VALUE_INIT;
@@ -705,6 +690,7 @@ gnc_budget_set_account_period_note(GncBudget *budget, const Account *account,
 
         qof_instance_set_kvp (QOF_INSTANCE (budget), &v, 3, GNC_BUDGET_NOTES_PATH, path_part_one, path_part_two);
         g_value_unset (&v);
+        perioddata.note = note;
     }
     qof_instance_set_dirty(&budget->inst);
     gnc_budget_commit_edit(budget);
@@ -713,21 +699,22 @@ gnc_budget_set_account_period_note(GncBudget *budget, const Account *account,
 
 }
 
-static gchar *
+static std::string
 get_account_period_note (const GncBudget *budget,
                          const Account *account, guint period_num)
 {
     gchar path_part_one [GUID_ENCODING_LENGTH + 1];
     gchar path_part_two [GNC_BUDGET_MAX_NUM_PERIODS_DIGITS];
     GValue v = G_VALUE_INIT;
-    gchar *retval;
+    std::string retval;
 
-    g_return_val_if_fail(GNC_IS_BUDGET(budget), NULL);
+    g_return_val_if_fail (GNC_IS_BUDGET(budget), "");
-    g_return_val_if_fail(account, NULL);
+    g_return_val_if_fail (account, "");
 
     make_period_path (account, period_num, path_part_one, path_part_two);
     qof_instance_get_kvp (QOF_INSTANCE (budget), &v, 3, GNC_BUDGET_NOTES_PATH, path_part_one, path_part_two);
-    retval = G_VALUE_HOLDS_STRING (&v) ? g_value_dup_string(&v) : NULL;
+    if (G_VALUE_HOLDS_STRING (&v))
+        retval = g_value_get_string(&v);
     g_value_unset (&v);
     return retval;
 }
@@ -737,9 +724,9 @@ gchar *
 gnc_budget_get_account_period_note (const GncBudget *budget,
                                     const Account *account, guint period_num)
 {
-    GArray *array = get_acct_array (budget, account);
+    g_return_val_if_fail (period_num < GET_PRIVATE(budget)->num_periods, nullptr);
-    PeriodData *data = (PeriodData*) &g_array_index (array, PeriodData, period_num);
+    auto& data = get_perioddata (budget, account, period_num);
-    return g_strdup (data->note);
+    return data.note.empty () ? nullptr : g_strdup (data.note.c_str());
 }
 
 time64
@@ -766,31 +753,36 @@ gnc_budget_get_account_period_actual_value(
                                            acc, period_num);
 }
 
-static GArray*
+static PeriodData&
-get_acct_array (const GncBudget *budget, const Account *account)
+get_perioddata (const GncBudget *budget, const Account *account, guint period_num)
 {
     GncBudgetPrivate *priv = GET_PRIVATE (budget);
-    GArray *array;
 
-    if (!g_hash_table_lookup_extended (priv->acct_hash, account, NULL,
+    if (period_num >= priv->num_periods)
-                                       (gpointer*)(&array)))
+        throw std::out_of_range("period_num >= num_periods");
+
+    auto& map = priv->acct_map;
+    auto map_iter = map->find (account);
+
+    if (map_iter == map->end ())
     {
-        array = g_array_sized_new (FALSE, TRUE, sizeof (PeriodData),
+        PeriodDataVec vec {};
-                                   priv->num_periods);
+        vec.reserve (priv->num_periods);
-        g_hash_table_insert (priv->acct_hash, (gpointer)account, array);
 
         for (guint i = 0; i < priv->num_periods; i++)
         {
-            PeriodData *data = (PeriodData*) &g_array_index (array, PeriodData, i);
+            PeriodData data {};
-            data->note = get_account_period_note (budget, account, i);
+            data.note = get_account_period_note (budget, account, i);
-            data->value_is_set = is_account_period_value_set (budget, account, i);
+            data.value_is_set = is_account_period_value_set (budget, account, i);
-
+            if (data.value_is_set)
-            if (data->value_is_set)
+                data.value = get_account_period_value (budget, account, i);
-                data->value = get_account_period_value (budget, account, i);
+            vec.push_back (std::move(data));
         }
+        map_iter = map->insert_or_assign(account, std::move(vec)).first;
     }
 
-    return array;
+    auto& vec = map_iter->second;
+    return vec.at(period_num);
 }
 
 GncBudget*

libgnucash/engine/test/utest-Budget.c

@@ -96,10 +96,10 @@ test_gnc_set_budget_num_periods_data_retention ()
     gnc_budget_set_num_periods(budget, 10);
     gnc_budget_set_num_periods(budget, 20);
 
-    /* value and note are retained */
+    /* value and note are lost */
-    g_assert (gnc_budget_is_account_period_value_set(budget, acc, 15));
+    g_assert (!gnc_budget_is_account_period_value_set(budget, acc, 15));
     note = gnc_budget_get_account_period_note (budget, acc, 11);
-    g_assert_cmpstr (note, ==, "undefined");
+    g_assert_cmpstr (note, ==, NULL);
     g_free (note);
 
     gnc_budget_destroy(budget);