From 8bc080b96dd13bf89376245c214e692173b57acb Mon Sep 17 00:00:00 2001 From: Christopher Lam Date: Wed, 12 Jan 2022 22:11:36 +0800 Subject: [PATCH 1/6] [gnc-budget] store budget kvp data onto memory upon first call. then each getter will retrieve from memory rather than backend's kvp. --- libgnucash/engine/gnc-budget.c | 141 ++++++++++++++++++++++++++++++--- 1 file changed, 130 insertions(+), 11 deletions(-) diff --git a/libgnucash/engine/gnc-budget.c b/libgnucash/engine/gnc-budget.c index db4b3ec5d3..a6e994a15f 100644 --- a/libgnucash/engine/gnc-budget.c +++ b/libgnucash/engine/gnc-budget.c @@ -69,6 +69,8 @@ typedef struct GncBudgetPrivate /* Recurrence (period info) for the budget */ Recurrence recurrence; + GHashTable *acct_hash; + /* Number of periods */ guint num_periods; } GncBudgetPrivate; @@ -84,6 +86,24 @@ 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) { @@ -94,6 +114,9 @@ gnc_budget_init(GncBudget* 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->num_periods = 12; date = gnc_g_date_new_today (); g_date_subtract_days(date, g_date_get_day(date) - 1); @@ -263,6 +286,8 @@ gnc_budget_free(QofInstance *inst) CACHE_REMOVE(priv->name); CACHE_REMOVE(priv->description); + g_hash_table_destroy (priv->acct_hash); + /* qof_instance_release (&budget->inst); */ g_object_unref(budget); } @@ -446,6 +471,13 @@ 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) { @@ -461,6 +493,9 @@ 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)); + qof_event_gen( &budget->inst, QOF_EVENT_MODIFY, NULL); } @@ -481,17 +516,27 @@ 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); + + /* period_num is zero-based */ /* What happens when account is deleted, after we have an entry for it? */ 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); + + acct_array = get_acct_array (budget, account); + perioddata = &g_array_index (acct_array, PeriodData, period_num); + perioddata->value_is_set = FALSE; + make_period_path (account, period_num, path_part_one, path_part_two); gnc_budget_begin_edit(budget); @@ -511,6 +556,8 @@ 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 */ @@ -523,11 +570,17 @@ 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); + make_period_path (account, period_num, path_part_one, path_part_two); gnc_budget_begin_edit(budget); 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; + } else { GValue v = G_VALUE_INIT; @@ -535,6 +588,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; } qof_instance_set_dirty(&budget->inst); gnc_budget_commit_edit(budget); @@ -546,10 +601,10 @@ gnc_budget_set_account_period_value(GncBudget *budget, const Account *account, /* We don't need these here, but maybe they're useful somewhere else? Maybe this should move to Account.h */ -gboolean -gnc_budget_is_account_period_value_set(const GncBudget *budget, - const Account *account, - guint period_num) +static gboolean +is_account_period_value_set (const GncBudget *budget, + const Account *account, + guint period_num) { GValue v = G_VALUE_INIT; gchar path_part_one [GUID_ENCODING_LENGTH + 1]; @@ -567,10 +622,20 @@ gnc_budget_is_account_period_value_set(const GncBudget *budget, return (ptr != NULL); } -gnc_numeric -gnc_budget_get_account_period_value(const GncBudget *budget, - const Account *account, - guint period_num) +gboolean +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; +} + +static gnc_numeric +get_account_period_value (const GncBudget *budget, + const Account *account, + guint period_num) { gnc_numeric *numeric = NULL; gnc_numeric retval; @@ -591,6 +656,15 @@ gnc_budget_get_account_period_value(const GncBudget *budget, return retval; } +gnc_numeric +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 (); +} void gnc_budget_set_account_period_note(GncBudget *budget, const Account *account, @@ -598,6 +672,8 @@ 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 */ @@ -610,9 +686,15 @@ 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); + 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); else @@ -631,9 +713,9 @@ gnc_budget_set_account_period_note(GncBudget *budget, const Account *account, } -gchar * -gnc_budget_get_account_period_note(const GncBudget *budget, - const Account *account, guint period_num) +static gchar * +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]; @@ -650,6 +732,16 @@ gnc_budget_get_account_period_note(const GncBudget *budget, return retval; } + +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); +} + time64 gnc_budget_get_period_start_date(const GncBudget *budget, guint period_num) { @@ -674,6 +766,33 @@ gnc_budget_get_account_period_actual_value( acc, period_num); } +static GArray* +get_acct_array (const GncBudget *budget, const Account *account) +{ + GncBudgetPrivate *priv = GET_PRIVATE (budget); + GArray *array; + + if (!g_hash_table_lookup_extended (priv->acct_hash, account, NULL, + (gpointer) &array)) + { + array = g_array_sized_new (FALSE, TRUE, sizeof (PeriodData), + 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); + 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); + } + } + + return array; +} + GncBudget* gnc_budget_lookup (const GncGUID *guid, const QofBook *book) { From 9088acabd866ee50b1c6c620569368bb46079082 Mon Sep 17 00:00:00 2001 From: Christopher Lam Date: Thu, 13 Jan 2022 18:10:38 +0800 Subject: [PATCH 2/6] [gnc-budget.cpp] convert to c++ --- libgnucash/engine/CMakeLists.txt | 2 +- libgnucash/engine/{gnc-budget.c => gnc-budget.cpp} | 12 ++++++------ libgnucash/engine/gnc-budget.h | 10 ++++++++++ po/POTFILES.in | 2 +- 4 files changed, 18 insertions(+), 8 deletions(-) rename libgnucash/engine/{gnc-budget.c => gnc-budget.cpp} (98%) diff --git a/libgnucash/engine/CMakeLists.txt b/libgnucash/engine/CMakeLists.txt index d860bd4361..8dde8461bc 100644 --- a/libgnucash/engine/CMakeLists.txt +++ b/libgnucash/engine/CMakeLists.txt @@ -139,7 +139,7 @@ set (engine_SOURCES cap-gains.c cashobjects.c gnc-aqbanking-templates.cpp - gnc-budget.c + gnc-budget.cpp gnc-commodity.c gnc-date.cpp gnc-datetime.cpp diff --git a/libgnucash/engine/gnc-budget.c b/libgnucash/engine/gnc-budget.cpp similarity index 98% rename from libgnucash/engine/gnc-budget.c rename to libgnucash/engine/gnc-budget.cpp index a6e994a15f..47f09e0ff2 100644 --- a/libgnucash/engine/gnc-budget.c +++ b/libgnucash/engine/gnc-budget.cpp @@ -33,6 +33,7 @@ #include "Account.h" +#include "guid.hpp" #include "gnc-budget.h" #include "gnc-commodity.h" @@ -196,7 +197,7 @@ gnc_budget_set_property( GObject* object, gnc_budget_set_num_periods(budget, g_value_get_uint(value)); break; case PROP_RECURRENCE: - gnc_budget_set_recurrence(budget, g_value_get_pointer(value)); + gnc_budget_set_recurrence (budget, static_cast(g_value_get_pointer(value))); break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); @@ -311,12 +312,11 @@ gnc_budget_commit_edit(GncBudget *bgt) GncBudget* gnc_budget_new(QofBook *book) { - GncBudget* budget; - g_return_val_if_fail(book, NULL); ENTER(" "); - budget = g_object_new(GNC_TYPE_BUDGET, NULL); + + auto budget { static_cast(g_object_new(GNC_TYPE_BUDGET, nullptr)) }; qof_instance_init_data (&budget->inst, GNC_ID_BUDGET, book); qof_event_gen( &budget->inst, QOF_EVENT_CREATE , NULL); @@ -773,7 +773,7 @@ get_acct_array (const GncBudget *budget, const Account *account) GArray *array; if (!g_hash_table_lookup_extended (priv->acct_hash, account, NULL, - (gpointer) &array)) + (gpointer*)(&array))) { array = g_array_sized_new (FALSE, TRUE, sizeof (PeriodData), priv->num_periods); @@ -879,7 +879,7 @@ static QofObject budget_object_def = DI(.interface_version = ) QOF_OBJECT_VERSION, DI(.e_type = ) GNC_ID_BUDGET, DI(.type_label = ) "Budget", - DI(.create = ) (gpointer)gnc_budget_new, + DI(.create = ) (void*(*)(QofBook*)) gnc_budget_new, DI(.book_begin = ) NULL, DI(.book_end = ) gnc_budget_book_end, DI(.is_dirty = ) qof_collection_is_dirty, diff --git a/libgnucash/engine/gnc-budget.h b/libgnucash/engine/gnc-budget.h index f5b9a2cee2..d9c1e814d5 100644 --- a/libgnucash/engine/gnc-budget.h +++ b/libgnucash/engine/gnc-budget.h @@ -64,6 +64,11 @@ #ifndef __GNC_BUDGET_H__ #define __GNC_BUDGET_H__ +#ifdef __cplusplus +extern "C" +{ +#endif + #include /** The budget data.*/ @@ -171,6 +176,11 @@ GncBudget* gnc_budget_get_default(QofBook *book); GncBudget* gnc_budget_lookup (const GncGUID *guid, const QofBook *book); #define gnc_budget_lookup_direct(g,b) gnc_budget_lookup(&(g),(b)) +#ifdef __cplusplus +} +#endif + + #endif // __BUDGET_H__ /** @} */ diff --git a/po/POTFILES.in b/po/POTFILES.in index 365d56798f..2194e9e5e3 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -630,7 +630,7 @@ libgnucash/engine/engine-helpers.c libgnucash/engine/gncAddress.c libgnucash/engine/gnc-aqbanking-templates.cpp libgnucash/engine/gncBillTerm.c -libgnucash/engine/gnc-budget.c +libgnucash/engine/gnc-budget.cpp libgnucash/engine/gncBusiness.c libgnucash/engine/gnc-commodity.c libgnucash/engine/gnc-commodity.h From 6c4c2512db53fc6b874f2004b743539fc651a9d2 Mon Sep 17 00:00:00 2001 From: Christopher Lam Date: Thu, 13 Jan 2022 21:01:36 +0800 Subject: [PATCH 3/6] Use C++ STL instead of GLib --- libgnucash/engine/gnc-budget.cpp | 158 ++++++++++++-------------- libgnucash/engine/test/utest-Budget.c | 6 +- 2 files changed, 78 insertions(+), 86 deletions(-) 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); From 919f392c7a43bb5cc469dfc6ace75e4b10db24c4 Mon Sep 17 00:00:00 2001 From: Christopher Lam Date: Thu, 20 Jan 2022 21:34:30 +0800 Subject: [PATCH 4/6] Use kvp C++ interface rather than GValue --- libgnucash/engine/gnc-budget.cpp | 92 +++++++------------------------- 1 file changed, 18 insertions(+), 74 deletions(-) diff --git a/libgnucash/engine/gnc-budget.cpp b/libgnucash/engine/gnc-budget.cpp index 45692498bc..b69c6094ff 100644 --- a/libgnucash/engine/gnc-budget.cpp +++ b/libgnucash/engine/gnc-budget.cpp @@ -71,6 +71,7 @@ struct PeriodData using PeriodDataVec = std::vector; using AcctMap = std::unordered_map; +using StringVec = std::vector; typedef struct GncBudgetPrivate { @@ -583,30 +584,6 @@ gnc_budget_set_account_period_value(GncBudget *budget, const Account *account, } -/* We don't need these here, but maybe they're useful somewhere else? - Maybe this should move to Account.h */ - -static gboolean -is_account_period_value_set (const GncBudget *budget, - const Account *account, - guint period_num) -{ - GValue v = G_VALUE_INIT; - gchar path_part_one [GUID_ENCODING_LENGTH + 1]; - gchar path_part_two [GNC_BUDGET_MAX_NUM_PERIODS_DIGITS]; - gconstpointer ptr = NULL; - - g_return_val_if_fail(GNC_IS_BUDGET(budget), FALSE); - g_return_val_if_fail(account, FALSE); - - make_period_path (account, period_num, path_part_one, path_part_two); - qof_instance_get_kvp (QOF_INSTANCE (budget), &v, 2, path_part_one, path_part_two); - if (G_VALUE_HOLDS_BOXED (&v)) - ptr = g_value_get_boxed (&v); - g_value_unset (&v); - return (ptr != NULL); -} - gboolean gnc_budget_is_account_period_value_set (const GncBudget *budget, const Account *account, @@ -616,30 +593,6 @@ gnc_budget_is_account_period_value_set (const GncBudget *budget, return get_perioddata (budget, account, period_num).value_is_set; } -static gnc_numeric -get_account_period_value (const GncBudget *budget, - const Account *account, - guint period_num) -{ - gnc_numeric *numeric = NULL; - gnc_numeric retval; - gchar path_part_one [GUID_ENCODING_LENGTH + 1]; - gchar path_part_two [GNC_BUDGET_MAX_NUM_PERIODS_DIGITS]; - GValue v = G_VALUE_INIT; - - g_return_val_if_fail(GNC_IS_BUDGET(budget), gnc_numeric_zero()); - g_return_val_if_fail(account, gnc_numeric_zero()); - - make_period_path (account, period_num, path_part_one, path_part_two); - qof_instance_get_kvp (QOF_INSTANCE (budget), &v, 2, path_part_one, path_part_two); - if (G_VALUE_HOLDS_BOXED (&v)) - numeric = (gnc_numeric*)g_value_get_boxed (&v); - - retval = numeric ? *numeric : gnc_numeric_zero (); - g_value_unset (&v); - return retval; -} - gnc_numeric gnc_budget_get_account_period_value (const GncBudget *budget, const Account *account, @@ -699,27 +652,6 @@ gnc_budget_set_account_period_note(GncBudget *budget, const Account *account, } -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; - std::string retval; - - 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); - if (G_VALUE_HOLDS_STRING (&v)) - retval = g_value_get_string(&v); - g_value_unset (&v); - return retval; -} - - gchar * gnc_budget_get_account_period_note (const GncBudget *budget, const Account *account, guint period_num) @@ -766,16 +698,28 @@ get_perioddata (const GncBudget *budget, const Account *account, guint period_nu if (map_iter == map->end ()) { + auto budget_kvp { QOF_INSTANCE (budget)->kvp_data }; + std::string acct_guid { guid_to_string (xaccAccountGetGUID (account)) }; + PeriodDataVec vec {}; vec.reserve (priv->num_periods); for (guint i = 0; i < priv->num_periods; 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); + std::string period_str { std::to_string (i) }; + std::string note; + StringVec path1 { acct_guid, period_str }; + StringVec path2 { GNC_BUDGET_NOTES_PATH, acct_guid, period_str }; + auto kval1 { budget_kvp->get_slot (path1) }; + auto kval2 { budget_kvp->get_slot (path2) }; + + auto is_set = kval1 && kval1->get_type() == KvpValue::Type::NUMERIC; + auto num = is_set ? kval1->get() : gnc_numeric_zero (); + + if (kval2 && kval2->get_type() == KvpValue::Type::STRING) + note = kval2->get(); + + PeriodData data { std::move (note), is_set, num }; vec.push_back (std::move(data)); } map_iter = map->insert_or_assign(account, std::move(vec)).first; From 8f845df93479264eb78f6cfe0f239273dbdd7c5d Mon Sep 17 00:00:00 2001 From: Christopher Lam Date: Thu, 20 Jan 2022 22:24:20 +0800 Subject: [PATCH 5/6] Factor out make_period_[data|note]_path --- libgnucash/engine/gnc-budget.cpp | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/libgnucash/engine/gnc-budget.cpp b/libgnucash/engine/gnc-budget.cpp index b69c6094ff..60cc72403f 100644 --- a/libgnucash/engine/gnc-budget.cpp +++ b/libgnucash/engine/gnc-budget.cpp @@ -506,6 +506,22 @@ make_period_path (const Account *account, guint period_num, char *path1, char *p g_sprintf (path2, "%d", period_num); } +static inline StringVec +make_period_data_path (const Account *account, guint period_num) +{ + gnc::GUID acct_guid {*(xaccAccountGetGUID (account))}; + return { acct_guid.to_string(), std::to_string (period_num) }; +} + +static inline StringVec +make_period_note_path (const Account *account, guint period_num) +{ + StringVec path { GNC_BUDGET_NOTES_PATH }; + StringVec data_path { make_period_data_path (account, period_num) }; + std::move (data_path.begin(), data_path.end(), std::back_inserter (path)); + return path; +} + static PeriodData& get_perioddata (const GncBudget *budget, const Account *account, guint period_num); @@ -699,19 +715,15 @@ get_perioddata (const GncBudget *budget, const Account *account, guint period_nu if (map_iter == map->end ()) { auto budget_kvp { QOF_INSTANCE (budget)->kvp_data }; - std::string acct_guid { guid_to_string (xaccAccountGetGUID (account)) }; PeriodDataVec vec {}; vec.reserve (priv->num_periods); for (guint i = 0; i < priv->num_periods; i++) { - std::string period_str { std::to_string (i) }; std::string note; - StringVec path1 { acct_guid, period_str }; - StringVec path2 { GNC_BUDGET_NOTES_PATH, acct_guid, period_str }; - auto kval1 { budget_kvp->get_slot (path1) }; - auto kval2 { budget_kvp->get_slot (path2) }; + auto kval1 { budget_kvp->get_slot (make_period_data_path (account, i)) }; + auto kval2 { budget_kvp->get_slot (make_period_note_path (account, i)) }; auto is_set = kval1 && kval1->get_type() == KvpValue::Type::NUMERIC; auto num = is_set ? kval1->get() : gnc_numeric_zero (); From a47413860ad0b206081b39a2c0b571cadd0cd647 Mon Sep 17 00:00:00 2001 From: Christopher Lam Date: Thu, 20 Jan 2022 22:39:41 +0800 Subject: [PATCH 6/6] Factor out GValue access for setters and getters --- libgnucash/engine/gnc-budget.cpp | 51 ++++++++------------------------ 1 file changed, 13 insertions(+), 38 deletions(-) diff --git a/libgnucash/engine/gnc-budget.cpp b/libgnucash/engine/gnc-budget.cpp index 60cc72403f..02beeff82b 100644 --- a/libgnucash/engine/gnc-budget.cpp +++ b/libgnucash/engine/gnc-budget.cpp @@ -496,16 +496,6 @@ gnc_budget_get_num_periods(const GncBudget* budget) return GET_PRIVATE(budget)->num_periods; } -static inline void -make_period_path (const Account *account, guint period_num, char *path1, char *path2) -{ - const GncGUID *guid; - gchar *bufend; - guid = xaccAccountGetGUID (account); - guid_to_string_buff (guid, path1); - g_sprintf (path2, "%d", period_num); -} - static inline StringVec make_period_data_path (const Account *account, guint period_num) { @@ -532,9 +522,6 @@ void gnc_budget_unset_account_period_value(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]; - g_return_if_fail (budget != NULL); g_return_if_fail (account != NULL); g_return_if_fail (period_num < GET_PRIVATE(budget)->num_periods); @@ -542,10 +529,10 @@ gnc_budget_unset_account_period_value(GncBudget *budget, const Account *account, 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); - gnc_budget_begin_edit(budget); - qof_instance_set_kvp (QOF_INSTANCE (budget), NULL, 2, path_part_one, path_part_two); + auto path = make_period_data_path (account, period_num); + auto budget_kvp { QOF_INSTANCE (budget)->kvp_data }; + delete budget_kvp->set_path (path, nullptr); qof_instance_set_dirty(&budget->inst); gnc_budget_commit_edit(budget); @@ -559,9 +546,6 @@ void gnc_budget_set_account_period_value(GncBudget *budget, const Account *account, guint period_num, gnc_numeric val) { - gchar path_part_one [GUID_ENCODING_LENGTH + 1]; - gchar path_part_two [GNC_BUDGET_MAX_NUM_PERIODS_DIGITS]; - /* Watch out for an off-by-one error here: * period_num starts from 0 while num_periods starts from 1 */ if (period_num >= GET_PRIVATE(budget)->num_periods) @@ -574,22 +558,19 @@ gnc_budget_set_account_period_value(GncBudget *budget, const Account *account, g_return_if_fail (account != NULL); auto& perioddata = get_perioddata (budget, account, period_num); - - make_period_path (account, period_num, path_part_one, path_part_two); + auto budget_kvp { QOF_INSTANCE (budget)->kvp_data }; + auto path = make_period_data_path (account, period_num); gnc_budget_begin_edit(budget); if (gnc_numeric_check(val)) { - qof_instance_set_kvp (QOF_INSTANCE (budget), NULL, 2, path_part_one, path_part_two); + delete budget_kvp->set_path (path, nullptr); perioddata.value_is_set = false; } else { - GValue v = G_VALUE_INIT; - g_value_init (&v, GNC_TYPE_NUMERIC); - 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); + KvpValue* v = new KvpValue (val); + delete budget_kvp->set_path (path, v); perioddata.value_is_set = true; perioddata.value = val; } @@ -627,9 +608,6 @@ void gnc_budget_set_account_period_note(GncBudget *budget, const Account *account, guint period_num, const gchar *note) { - gchar path_part_one [GUID_ENCODING_LENGTH + 1]; - gchar path_part_two [GNC_BUDGET_MAX_NUM_PERIODS_DIGITS]; - /* Watch out for an off-by-one error here: * period_num starts from 0 while num_periods starts from 1 */ if (period_num >= GET_PRIVATE(budget)->num_periods) @@ -642,23 +620,20 @@ gnc_budget_set_account_period_note(GncBudget *budget, const Account *account, g_return_if_fail (account != NULL); auto& perioddata = get_perioddata (budget, account, period_num); - - make_period_path (account, period_num, path_part_one, path_part_two); + auto budget_kvp { QOF_INSTANCE (budget)->kvp_data }; + auto path = make_period_note_path (account, period_num); gnc_budget_begin_edit(budget); if (note == NULL) { - qof_instance_set_kvp (QOF_INSTANCE (budget), NULL, 3, GNC_BUDGET_NOTES_PATH, path_part_one, path_part_two); + delete budget_kvp->set_path (path, nullptr); perioddata.note.clear (); } else { - GValue v = G_VALUE_INIT; - g_value_init (&v, G_TYPE_STRING); - g_value_set_string (&v, note); + KvpValue* v = new KvpValue (g_strdup (note)); - qof_instance_set_kvp (QOF_INSTANCE (budget), &v, 3, GNC_BUDGET_NOTES_PATH, path_part_one, path_part_two); - g_value_unset (&v); + delete budget_kvp->set_path (path, v); perioddata.note = note; } qof_instance_set_dirty(&budget->inst);