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

[Jesse Olmer]

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.