> On Dec. 7, 2012, 6:35 p.m., Cristian Oneț wrote: > >
I don't have anything else to add other than these two issues and the boost hard dependency. If the dependency get's the OK from the others and these small issues are fixed then this review is OK to be shipped since it seems to work as the old code but it is a lot more readable. - Cristian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107137/#review23132 ----------------------------------------------------------- On Dec. 5, 2012, 8:28 p.m., Łukasz Maszczyński wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/107137/ > ----------------------------------------------------------- > > (Updated Dec. 5, 2012, 8:28 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 > >
_______________________________________________ KMyMoney-devel mailing list KMyMoney-devel@kde.org https://mail.kde.org/mailman/listinfo/kmymoney-devel
