diff --git a/libgnucash/engine/gnc-budget.cpp b/libgnucash/engine/gnc-budget.cpp index 47f09e0ff2..45692498bc 100644 --- a/libgnucash/engine/gnc-budget.cpp +++ b/libgnucash/engine/gnc-budget.cpp @@ -30,6 +30,9 @@ #include #include #include +#include +#include +#include #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; +using AcctMap = std::unordered_map; + 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 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_hash = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, - (GDestroyNotify) acct_array_free); + priv->acct_map = std::make_unique(); 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); - - g_hash_table_destroy (priv->acct_hash); + priv->acct_map = nullptr; // nullify to ensure unique_ptr is freed. /* 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, - GUINT_TO_POINTER (num_periods)); + std::for_each (priv->acct_map->begin(), + 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); - perioddata = &g_array_index (acct_array, PeriodData, period_num); - perioddata->value_is_set = FALSE; + auto& data = get_perioddata (budget, account, period_num); + data.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); - perioddata = &g_array_index (acct_array, PeriodData, period_num); + auto& perioddata = get_perioddata (budget, account, 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 = val; + perioddata.value_is_set = true; + 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); - PeriodData *data = (PeriodData*) &g_array_index (array, PeriodData, period_num); - return data->value_is_set; + g_return_val_if_fail (period_num < GET_PRIVATE(budget)->num_periods, false); + return get_perioddata (budget, account, period_num).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); - PeriodData *data = (PeriodData*) &g_array_index (array, PeriodData, period_num); - return data->value_is_set ? data->value : gnc_numeric_zero (); + g_return_val_if_fail (period_num < GET_PRIVATE(budget)->num_periods, + 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); - perioddata = &g_array_index (acct_array, PeriodData, period_num); + auto& perioddata = get_perioddata (budget, account, 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(account, NULL); + g_return_val_if_fail (GNC_IS_BUDGET(budget), ""); + 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); - PeriodData *data = (PeriodData*) &g_array_index (array, PeriodData, period_num); - return g_strdup (data->note); + g_return_val_if_fail (period_num < GET_PRIVATE(budget)->num_periods, nullptr); + auto& data = get_perioddata (budget, account, period_num); + 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* -get_acct_array (const GncBudget *budget, const Account *account) +static PeriodData& +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, - (gpointer*)(&array))) + if (period_num >= priv->num_periods) + 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), - priv->num_periods); - g_hash_table_insert (priv->acct_hash, (gpointer)account, array); + PeriodDataVec vec {}; + vec.reserve (priv->num_periods); for (guint i = 0; i < priv->num_periods; i++) { - PeriodData *data = (PeriodData*) &g_array_index (array, PeriodData, i); - data->note = get_account_period_note (budget, account, i); - data->value_is_set = is_account_period_value_set (budget, account, i); - - if (data->value_is_set) - data->value = get_account_period_value (budget, account, i); + PeriodData data {}; + data.note = get_account_period_note (budget, account, i); + data.value_is_set = is_account_period_value_set (budget, account, i); + if (data.value_is_set) + 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* diff --git a/libgnucash/engine/test/utest-Budget.c b/libgnucash/engine/test/utest-Budget.c index 7e8a2d94ae..f899b38a39 100644 --- a/libgnucash/engine/test/utest-Budget.c +++ b/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 */ - g_assert (gnc_budget_is_account_period_value_set(budget, acc, 15)); + /* value and note are lost */ + 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);