[Kmymoney-devel] Review Request: Refactoring of matching a transaction-under-import

[Łukasz Maszczyński]

> On Nov. 27, 2012, 8:51 p.m., Cristian Oneț wrote:
> > Today I have managed to make a functional test. It seems that there is a bug somewhere since repeatedly importing the same ofx file adds the same transactions over and over again although the import dialog states that the duplicates where found.
> > If you need the sample file let me know and I'll mail it to you. The current version correctly imports the transactions only once.

Confirmed - that is a change of behaviour compared to the original version. I fixed it in diff revision 4.


- Łukasz


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107137/#review22669
-----------------------------------------------------------


On Nov. 28, 2012, 8:19 p.m., Łukasz Maszczyński wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107137/
> -----------------------------------------------------------
> 
> (Updated Nov. 28, 2012, 8:19 p.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Description
> -------
> 
> 1. please note that dependency on Boost is no longer optional (see changes in CMakeLists.txt)
> 
> 2. Architectural changes
>    Until now, the method MyMoneyStatementReader::processTransactionEntry() handled the whole process of importing a transaction - that is: handling the securities, matching and creating payees, and - at the very end of the method - adding the transaction to the ledger.
>    The last step (adding transaction to ledger) is the main target of this refactoring. Its algorithm was as follows:
>    1. find a matching transaction (either existing or scheduled) - using TransactionMatcher::findMatch()
>    2. If an "existing transaction match" is found - handle it (in the old code it's the block starting with a comment "// it matched a simple transaction. that's the easy case")
>    3. Else if a "scheduled transaction match" is found - handle it ("// a match has been found in a pending schedule"...)
> 
>    Code "mapping" is as follows:
>    - step 2 (above) is extracted to handleMatchingOfExistingTransaction()
>    - step 3 (above) is extracted to handleMatchingOfScheduledTransaction()
>    - TransactionMatcher::findMatch() is extracted to TransactionMatchFinder::findMatch() (note: there are two pure-virtual functions that are implemented in ExistingTransactionMatchFinder, ScheduledTransactionMatchFinder classes)
>    - TransactionMatcher::checkTransaction() is extracted to TransactionMatchFinder::findMatchingSplit()
> 
> 3. Memory management changes
>    Raw pointers are no longer used, as these are typically error-prone. Pointers were replaced either with object instances, or boost::optional is used if applicable (e.g. see members of TransactionMatchFinder class).
> 
> 4. dynamic_casts removed (were used on pointers returned by TransactionMatcher::findMatch(), no longer needed - see TransactionMatchFinder::getMatchedTransaction() and getMatchedSchedule() )
> 
> 5. variable/method names - I did my best to keep those meaningful: e.g. "importedTransaction" instead of "t")
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 93af070 
>   kmymoney/converter/mymoneystatementreader.h 758ff00 
>   kmymoney/converter/mymoneystatementreader.cpp 42c4841 
>   kmymoney/dialogs/CMakeLists.txt 9a8d782 
>   kmymoney/dialogs/existingtransactionmatchfinder.h PRE-CREATION 
>   kmymoney/dialogs/existingtransactionmatchfinder.cpp PRE-CREATION 
>   kmymoney/dialogs/scheduledtransactionmatchfinder.h PRE-CREATION 
>   kmymoney/dialogs/scheduledtransactionmatchfinder.cpp PRE-CREATION 
>   kmymoney/dialogs/transactionmatcher.h d09a4cd 
>   kmymoney/dialogs/transactionmatcher.cpp c380877 
>   kmymoney/dialogs/transactionmatchfinder.h PRE-CREATION 
>   kmymoney/dialogs/transactionmatchfinder.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/107137/diff/
> 
> 
> Testing
> -------
> 
> make test
> 
> 
> Thanks,
> 
> Łukasz Maszczyński
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kmymoney-devel/attachments/20121128/6b0c1cbb/attachment-0001.html>