Bug 739571 - Matching imported transactions doesn't indicate previously matched entries

[John Ralls]

> On Jan 23, 2016, at 10:30 PM, Jesse Olmer <jesse at wickedgoodtimes.com> wrote:
> 
> On Sat, Jan 23, 2016 at 10:13 PM, John Ralls <jralls at ceridwen.us> wrote:
>> 
>>> On Jan 23, 2016, at 9:44 PM, Jesse Olmer <jesse at wickedgoodtimes.com> wrote:
>>> 
>>> 
>>> Yeah that's essentially what I'm doing so far (although it'll need a
>>> bit of a rewrite if C++ isn't cool in maint). Instead of a hash table
>>> I'm using an unordered_map<GUID,
>>> TypeWhichDoesRefCountAndMatchTypeIndicator> which lives in
>>> _main_matcher_info and gets passed in to each match picker instance
>>> via gnc_import_match_picker_run_and_close. Filtering & display happens
>>> where you'd expect in match_update_match_model, and refcount updates
>>> are handled in the result of gnc_import_match_picker_run_and_close.
>>> Having to switch to C will take me about a day but hopefully I'll have
>>> something ready tomorrow.
>> 
>> 
>> I imagine that you're putting every proposed-match transaction's GUID in the hashtable with a flag to indicate whether it's been used or not and a ref count because you're putting a pointer in match_infos or something. I don't think you need to do that. Just add the GUID to the hashtable when the user selects the transaction for a match (and remove it if she unselects it). Then you need only do a lookup when populating the GtkListModel: If it succeeds then the transaction has already been used, if it fails it hasn't. For C++ this has the added advantage of being able to use the simpler std::vector<GUID*> and std::find instead of std::unordered_map.
>> 
>> A couple of notes on ref counting. First, don't ever roll your own. In C++ use std::shared_ptr to wrap a ptr that needs ref counting. In C with GObject, derive the class that needs ref counting from GObject. Second, heed the new C++ Core Guidlines (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md) and Stroustroup's recent teaching on smart ptrs: They're only needed when (potential) transfer of ownership is involved. That's not the case here with the GUID ptrs; the Transaction or Split retains ownership and we can be confident that they won't be destroyed while we're fiddling with the import, so there's no need to free them nor to worry about them dangling. Raw ptrs are fine.
>> 
> 
> ref count was a poor choice of words on my part. I'm aware of smart
> pointers (and that they're not of any help here). What I was referring
> to, instead, was that I need some way to track the number of
> transactions which are associated with a given match. Perhaps an
> example would clarify my thinking here:
> 
> ImportA has an automatic best match to Trans1. I write into the table
> Trans1's guid as the key, and 1 as the value
> ImportB has an automatic best match to Trans1. I update the table
> Trans1's guid as the key, 2 as the value
> The User changes ImportA to match Trans2. I update the table twice:
> {Trans1, 1}, {Trans2, 1}
> 
> The value in this example is an int as gpointer (using the appropriate
> glib macros) specifically used in the case where 1 of many Imports are
> moved to a new match, but the old match is still a pending match. In
> reality the value I'm using is a bit more complex since it also tracks
> manual vs. automatic match for each Import so that this information
> can be presented to the user rather than just hiding these matches
> (otherwise the auto-matches would make it pretty hard to change things
> around). I'm not actually keeping track of / a pointer to any other
> object type in the system. Hopefully that clears up my intent a bit
> more until I can get a patch ready, and if not worst case is I spend
> an hour or two tomorrow wasting my time trying it out.

Hmm. ISTM that's going a bit beyond what the bug asks for, but maybe I'm not thinking through the situation fully. How are you going to present this information to the user? Are you going to change the matcher logic to take it into account, so that if there are (say) 3 similar existing transactions and 3 imported transactions that match them with equal scores the matcher will assign one of the imported transactions to each of the existing transactions?

Regards,
John Ralls