IntenseFinance/gnucash
3
0
Fork 0
mirror of https://github.com/Gnucash/gnucash.git synced 2024-11-30 04:33:55 -06:00
Code Issues Packages Projects Releases Wiki Activity

Code review responses

Browse Source 
Using Aliases to represent cmplicated types
Corrected variable-sized array on stack
Using PascalCase for type names and aliases
This commit is contained in:
lmat 2017-12-06 13:52:01 -08:00
parent 3312fe2dcd
commit f782be1a51
3 changed files with 49 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

95
libgnucash/engine/Account.cpp
View File

@ -58,6 +58,10 @@ static const char *KEY_ASSOC_INCOME_ACCOUNT = "ofx/associated-income-account";
#define AB_BANK_CODE "bank-code" #define AB_BANK_CODE "bank-code"
#define AB_TRANS_RETRIEVAL "trans-retrieval" #define AB_TRANS_RETRIEVAL "trans-retrieval"
using FinalProbabilityVec=std::vector<std::pair<std::string, int32_t>>;
using ProbabilityVec=std::vector<std::pair<std::string, struct AccountProbability>>;
using FlatKvpEntry=std::pair<std::string, KvpValue*>;
enum enum
{ {
LAST_SIGNAL LAST_SIGNAL
@ -3027,6 +3031,7 @@ gnc_account_get_full_name(const Account *account)
AccountPrivate *priv; AccountPrivate *priv;
const Account *a; const Account *a;
char *fullname; char *fullname;
gchar **names;
int level; int level;
/* So much for hardening the API. Too many callers to this function don't /* So much for hardening the API. Too many callers to this function don't
@ -3053,7 +3058,7 @@ gnc_account_get_full_name(const Account *account)
/* Get all the pointers in the right order. The root node "entry" /* Get all the pointers in the right order. The root node "entry"
* becomes the terminating NULL pointer for the array of strings. */ * becomes the terminating NULL pointer for the array of strings. */
gchar* names[level*sizeof(gchar*)]; names = (gchar **)g_malloc(level * sizeof(gchar *));
names[--level] = NULL; names[--level] = NULL;
for (a = account; level > 0; a = priv->parent) for (a = account; level > 0; a = priv->parent)
{ {
@ -3063,6 +3068,7 @@ gnc_account_get_full_name(const Account *account)
/* Build the full name */ /* Build the full name */
fullname = g_strjoinv(account_separator, names); fullname = g_strjoinv(account_separator, names);
g_free(names);
return fullname; return fullname;
} }
@ -5151,13 +5157,13 @@ gnc_account_imap_delete_account (GncImportMatchMap *imap,
where p(AB) = (a*b)/[a*b + (1-a)(1-b)], product is (a*b), where p(AB) = (a*b)/[a*b + (1-a)(1-b)], product is (a*b),
product_difference is (1-a) * (1-b) product_difference is (1-a) * (1-b)
*/ */
struct account_probability struct AccountProbability
{ {
double product; /* product of probabilities */ double product; /* product of probabilities */
double product_difference; /* product of (1-probabilities) */ double product_difference; /* product of (1-probabilities) */
}; };
struct account_token_count struct AccountTokenCount
{ {
std::string account_guid; std::string account_guid;
int64_t token_count; /** occurrences of a given token for this account_guid */ int64_t token_count; /** occurrences of a given token for this account_guid */
@ -5166,26 +5172,26 @@ struct account_token_count
/** total_count and the token_count for a given account let us calculate the /** total_count and the token_count for a given account let us calculate the
* probability of a given account with any single token * probability of a given account with any single token
*/ */
struct token_accounts_info struct TokenAccountsInfo
{ {
std::vector<account_token_count> accounts; std::vector<AccountTokenCount> accounts;
int64_t total_count; int64_t total_count;
}; };
/** holds an account guid and its corresponding integer probability /** holds an account guid and its corresponding integer probability
the integer probability is some factor of 10 the integer probability is some factor of 10
*/ */
struct account_info struct AccountInfo
{ {
std::string account_guid; std::string account_guid;
int32_t probability; int32_t probability;
}; };
static void static void
build_token_info(char const * key, KvpValue * value, token_accounts_info & tokenInfo) build_token_info(char const * key, KvpValue * value, TokenAccountsInfo & tokenInfo)
{ {
tokenInfo.total_count += value->get<int64_t>(); tokenInfo.total_count += value->get<int64_t>();
account_token_count this_account; AccountTokenCount this_account;
std::string account_guid {key}; std::string account_guid {key};
/*By convention, the key ends with the account GUID.*/ /*By convention, the key ends with the account GUID.*/
this_account.account_guid = account_guid.substr(account_guid.size() - GUID_ENCODING_LENGTH); this_account.account_guid = account_guid.substr(account_guid.size() - GUID_ENCODING_LENGTH);
@ -5198,10 +5204,10 @@ build_token_info(char const * key, KvpValue * value, token_accounts_info & token
0.10 * 100000 = 10000 */ 0.10 * 100000 = 10000 */
static constexpr int probability_factor = 100000; static constexpr int probability_factor = 100000;
static std::vector<std::pair<std::string, int32_t>> static FinalProbabilityVec
build_probabilities(std::vector<std::pair<std::string, account_probability>> const & first_pass) build_probabilities(ProbabilityVec const & first_pass)
{ {
std::vector<std::pair<std::string, int32_t>> ret; FinalProbabilityVec ret;
for (auto const & first_pass_prob : first_pass) for (auto const & first_pass_prob : first_pass)
{ {
auto const & account_probability = first_pass_prob.second; auto const & account_probability = first_pass_prob.second;
@ -5216,31 +5222,31 @@ build_probabilities(std::vector<std::pair<std::string, account_probability>> con
return ret; return ret;
} }
static account_info static AccountInfo
highest_probability(std::vector<std::pair<std::string, int32_t>> const & probabilities) highest_probability(FinalProbabilityVec const & probabilities)
{ {
account_info ret {"", std::numeric_limits<int32_t>::min()}; AccountInfo ret {"", std::numeric_limits<int32_t>::min()};
for (auto const & prob : probabilities) for (auto const & prob : probabilities)
if (prob.second > ret.probability) if (prob.second > ret.probability)
ret = account_info{prob.first, prob.second}; ret = AccountInfo {prob.first, prob.second};
return ret; return ret;
} }
static std::vector<std::pair<std::string, account_probability>> static ProbabilityVec
get_first_pass_probabilities(GncImportMatchMap * imap, GList * tokens) get_first_pass_probabilities(GncImportMatchMap * imap, GList * tokens)
{ {
std::vector<std::pair<std::string, account_probability>> ret; ProbabilityVec ret;
/* find the probability for each account that contains any of the tokens /* find the probability for each account that contains any of the tokens
* in the input tokens list. */ * in the input tokens list. */
for (auto current_token = tokens; current_token; current_token = current_token->next) for (auto current_token = tokens; current_token; current_token = current_token->next)
{ {
token_accounts_info tokenInfo{}; TokenAccountsInfo tokenInfo{};
auto path = std::string{IMAP_FRAME_BAYES "/"} + static_cast <char const *> (current_token->data); auto path = std::string{IMAP_FRAME_BAYES "/"} + static_cast <char const *> (current_token->data);
qof_instance_foreach_slot_prefix (QOF_INSTANCE (imap->acc), path, &build_token_info, tokenInfo); qof_instance_foreach_slot_prefix (QOF_INSTANCE (imap->acc), path, &build_token_info, tokenInfo);
for (auto const & current_account_token : tokenInfo.accounts) for (auto const & current_account_token : tokenInfo.accounts)
{ {
auto item = std::find_if(ret.begin(), ret.end(), [&current_account_token] auto item = std::find_if(ret.begin(), ret.end(), [&current_account_token]
(std::pair<std::string, account_probability> const & a) { (std::pair<std::string, AccountProbability> const & a) {
return current_account_token.account_guid == a.first; return current_account_token.account_guid == a.first;
}); });
if (item != ret.end()) if (item != ret.end())
@ -5253,7 +5259,7 @@ get_first_pass_probabilities(GncImportMatchMap * imap, GList * tokens)
else else
{ {
/* add a new entry */ /* add a new entry */
account_probability new_probability; AccountProbability new_probability;
new_probability.product = ((double)current_account_token.token_count / new_probability.product = ((double)current_account_token.token_count /
(double)tokenInfo.total_count); (double)tokenInfo.total_count);
new_probability.product_difference = 1 - (new_probability.product); new_probability.product_difference = 1 - (new_probability.product);
@ -5385,16 +5391,11 @@ build_non_bayes (const char *key, const GValue *value, gpointer user_data)
{ {
if (!G_VALUE_HOLDS_BOXED (value)) if (!G_VALUE_HOLDS_BOXED (value))
return; return;
QofBook *book; QofBook *book;
GncGUID *guid = NULL; GncGUID *guid = NULL;
gchar *kvp_path; gchar *kvp_path;
gchar *guid_string = NULL; gchar *guid_string = NULL;
auto imapInfo = (GncImapInfo*)user_data;
struct imap_info *imapInfo_node;
struct imap_info *imapInfo = (struct imap_info*)user_data;
// Get the book // Get the book
book = qof_instance_get_book (imapInfo->source_account); book = qof_instance_get_book (imapInfo->source_account);
@ -5408,7 +5409,7 @@ build_non_bayes (const char *key, const GValue *value, gpointer user_data)
PINFO("build_non_bayes: kvp_path is '%s'", kvp_path); PINFO("build_non_bayes: kvp_path is '%s'", kvp_path);
imapInfo_node = static_cast <imap_info*> (g_malloc(sizeof(*imapInfo_node))); auto imapInfo_node = static_cast <GncImapInfo*> (g_malloc(sizeof(GncImapInfo)));
imapInfo_node->source_account = imapInfo->source_account; imapInfo_node->source_account = imapInfo->source_account;
imapInfo_node->map_account = xaccAccountLookup (guid, book); imapInfo_node->map_account = xaccAccountLookup (guid, book);
@ -5435,7 +5436,7 @@ parse_bayes_imap_info (std::string const & imap_bayes_entry)
} }
static void static void
build_bayes (const char *key, KvpValue * value, imap_info & imapInfo) build_bayes (const char *key, KvpValue * value, GncImapInfo & imapInfo)
{ {
auto slots = qof_instance_get_slots_prefix (QOF_INSTANCE (imapInfo.source_account), IMAP_FRAME_BAYES); auto slots = qof_instance_get_slots_prefix (QOF_INSTANCE (imapInfo.source_account), IMAP_FRAME_BAYES);
if (!slots.size()) return; if (!slots.size()) return;
@ -5446,7 +5447,7 @@ build_bayes (const char *key, KvpValue * value, imap_info & imapInfo)
GncGUID guid = temp_guid; GncGUID guid = temp_guid;
auto map_account = xaccAccountLookup (&guid, gnc_account_get_book (imapInfo.source_account)); auto map_account = xaccAccountLookup (&guid, gnc_account_get_book (imapInfo.source_account));
std::string category_head {std::get <0> (parsed_key) + "/" + std::get <1> (parsed_key)}; std::string category_head {std::get <0> (parsed_key) + "/" + std::get <1> (parsed_key)};
auto imap_node = static_cast <imap_info*> (g_malloc (sizeof (imap_info))); auto imap_node = static_cast <GncImapInfo*> (g_malloc (sizeof (GncImapInfo)));
auto count = entry.second->get <int64_t> (); auto count = entry.second->get <int64_t> ();
imap_node->source_account = imapInfo.source_account; imap_node->source_account = imapInfo.source_account;
imap_node->map_account = map_account; imap_node->map_account = map_account;
@ -5534,29 +5535,23 @@ gnc_account_delete_map_entry (Account *acc, char *full_category, gboolean empty)
/*******************************************************************************/ /*******************************************************************************/
static gchar * static std::string
look_for_old_separator_descendants (Account *root, gchar const *full_name, const gchar *separator) look_for_old_separator_descendants (Account *root, std::string const & full_name, const gchar *separator)
{ {
GList *top_accounts, *ptr; GList *top_accounts, *ptr;
gint found_len = 0; gint found_len = 0;
gchar found_sep; gchar found_sep;
gchar * new_name = nullptr;
top_accounts = gnc_account_get_descendants (root); top_accounts = gnc_account_get_descendants (root);
PINFO("Incoming full_name is '%s', current separator is '%s'", full_name.c_str (), separator);
PINFO("Incoming full_name is '%s', current separator is '%s'", full_name, separator);
/* Go through list of top level accounts */ /* Go through list of top level accounts */
for (ptr = top_accounts; ptr; ptr = g_list_next (ptr)) for (ptr = top_accounts; ptr; ptr = g_list_next (ptr))
{ {
const gchar *name = xaccAccountGetName (static_cast <Account const *> (ptr->data)); const gchar *name = xaccAccountGetName (static_cast <Account const *> (ptr->data));
// we are looking for the longest top level account that matches // we are looking for the longest top level account that matches
if (g_str_has_prefix (full_name, name)) if (g_str_has_prefix (full_name.c_str (), name))
{ {
gint name_len = strlen (name); gint name_len = strlen (name);
const gchar old_sep = full_name[name_len]; const gchar old_sep = full_name[name_len];
if (!g_ascii_isalnum (old_sep)) // test for non alpha numeric if (!g_ascii_isalnum (old_sep)) // test for non alpha numeric
{ {
if (name_len > found_len) if (name_len > found_len)
@ -5568,13 +5563,10 @@ look_for_old_separator_descendants (Account *root, gchar const *full_name, const
} }
} }
g_list_free (top_accounts); // Free the List g_list_free (top_accounts); // Free the List
new_name = g_strdup (full_name); std::string new_name {full_name};
if (found_len > 1) if (found_len > 1)
g_strdelimit (new_name, &found_sep, *separator); std::replace (new_name.begin (), new_name.end (), found_sep, *separator);
PINFO("Return full_name is '%s'", new_name); PINFO("Return full_name is '%s'", new_name);
return new_name; return new_name;
} }
@ -5584,17 +5576,16 @@ get_guid_from_account_name (Account * root, std::string const & name)
auto map_account = gnc_account_lookup_by_full_name (root, name.c_str ()); auto map_account = gnc_account_lookup_by_full_name (root, name.c_str ());
if (!map_account) if (!map_account)
{ {
auto temp_account_name = look_for_old_separator_descendants (root, name.c_str (), auto temp_account_name = look_for_old_separator_descendants (root, name,
gnc_get_account_separator_string ()); gnc_get_account_separator_string ());
map_account = gnc_account_lookup_by_full_name (root, temp_account_name); map_account = gnc_account_lookup_by_full_name (root, temp_account_name.c_str ());
g_free (temp_account_name);
} }
auto temp_guid = gnc::GUID {*xaccAccountGetGUID (map_account)}; auto temp_guid = gnc::GUID {*xaccAccountGetGUID (map_account)};
return temp_guid.to_string (); return temp_guid.to_string ();
} }
static std::pair <std::string, KvpValue*> static FlatKvpEntry
convert_entry (std::pair <std::vector <std::string>, KvpValue*> entry, Account* root) convert_entry (KvpEntry entry, Account* root)
{ {
/*We need to make a copy here.*/ /*We need to make a copy here.*/
auto account_name = entry.first.back(); auto account_name = entry.first.back();
@ -5607,7 +5598,7 @@ convert_entry (std::pair <std::vector <std::string>, KvpValue*> entry, Account*
return {new_key, entry.second}; return {new_key, entry.second};
} }
static std::vector<std::pair<std::string, KvpValue*>> static std::vector<FlatKvpEntry>
get_new_guid_imap (Account * acc) get_new_guid_imap (Account * acc)
{ {
auto frame = qof_instance_get_slots (QOF_INSTANCE (acc)); auto frame = qof_instance_get_slots (QOF_INSTANCE (acc));
@ -5617,7 +5608,7 @@ get_new_guid_imap (Account * acc)
auto imap_frame = slot->get<KvpFrame*> (); auto imap_frame = slot->get<KvpFrame*> ();
auto flat_kvp = imap_frame->flatten_kvp (); auto flat_kvp = imap_frame->flatten_kvp ();
auto root = gnc_account_get_root (acc); auto root = gnc_account_get_root (acc);
std::vector <std::pair <std::string, KvpValue*>> ret; std::vector <FlatKvpEntry> ret;
for (auto const & flat_entry : flat_kvp) for (auto const & flat_entry : flat_kvp)
ret.emplace_back (convert_entry (flat_entry, root)); ret.emplace_back (convert_entry (flat_entry, root));
return ret; return ret;
@ -5637,7 +5628,7 @@ convert_imap_account_bayes_to_guid (Account *acc)
xaccAccountCommitEdit(acc); xaccAccountCommitEdit(acc);
return false; return false;
} }
std::for_each(new_imap.begin(), new_imap.end(), [&frame] (std::pair<std::string, KvpValue*> const & entry) { std::for_each(new_imap.begin(), new_imap.end(), [&frame] (FlatKvpEntry const & entry) {
frame->set({entry.first.c_str()}, entry.second); frame->set({entry.first.c_str()}, entry.second);
}); });
qof_instance_set_dirty (QOF_INSTANCE (acc)); qof_instance_set_dirty (QOF_INSTANCE (acc));

6
libgnucash/engine/kvp-frame.cpp
View File

@ -423,7 +423,7 @@ gnc_value_list_get_type (void)
} }
void void
KvpFrame::flatten_kvp_impl(std::vector <std::string> path, std::vector <std::pair <std::vector <std::string>, KvpValue*>> & entries) const noexcept KvpFrame::flatten_kvp_impl(std::vector <std::string> path, std::vector <KvpEntry> & entries) const noexcept
{ {
for (auto const & entry : m_valuemap) for (auto const & entry : m_valuemap)
{ {
@ -443,10 +443,10 @@ KvpFrame::flatten_kvp_impl(std::vector <std::string> path, std::vector <std::pai
} }
} }
std::vector <std::pair <std::vector <std::string>, KvpValue*>> std::vector <KvpEntry>
KvpFrame::flatten_kvp(void) const noexcept KvpFrame::flatten_kvp(void) const noexcept
{ {
std::vector <std::pair <std::vector <std::string>, KvpValue*>> ret; std::vector <KvpEntry> ret;
flatten_kvp_impl({}, ret); flatten_kvp_impl({}, ret);
return ret; return ret;
} }

5
libgnucash/engine/kvp-frame.hpp
View File

@ -92,6 +92,7 @@
#include <algorithm> #include <algorithm>
#include <iostream> #include <iostream>
using Path = std::vector<std::string>; using Path = std::vector<std::string>;
using KvpEntry = std::pair <std::vector <std::string>, KvpValue*>;
/** Implements KvpFrame. /** Implements KvpFrame.
* It's a struct because QofInstance needs to use the typename to declare a * It's a struct because QofInstance needs to use the typename to declare a
@ -216,7 +217,7 @@ struct KvpFrameImpl
* Returns all keys and values of this frame recursively, flattening * Returns all keys and values of this frame recursively, flattening
* the frame-containing values. * the frame-containing values.
*/ */
std::vector <std::pair <std::vector <std::string>, KvpValue*>> std::vector <KvpEntry>
flatten_kvp(void) const noexcept; flatten_kvp(void) const noexcept;
/** Test for emptiness /** Test for emptiness
@ -230,7 +231,7 @@ struct KvpFrameImpl
KvpFrame * get_child_frame_or_nullptr (Path const &) noexcept; KvpFrame * get_child_frame_or_nullptr (Path const &) noexcept;
KvpFrame * get_child_frame_or_create (Path const &) noexcept; KvpFrame * get_child_frame_or_create (Path const &) noexcept;
void flatten_kvp_impl(std::vector <std::string>, std::vector <std::pair <std::vector <std::string>, KvpValue*>> &) const noexcept; void flatten_kvp_impl(std::vector <std::string>, std::vector <KvpEntry> &) const noexcept;
KvpValue * set_impl (std::string const &, KvpValue *) noexcept; KvpValue * set_impl (std::string const &, KvpValue *) noexcept;
}; };