<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/107137/">http://git.reviewboard.kde.org/r/107137/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 26th, 2012, 4:56 p.m., <b>Cristian Oneț</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The patch looks really good, I still have to give it a try though until that I thought I'll publish these 'administrative' observations with the last one of them being: did you run astyle-kmymoney.sh (in the source tree, needs astyle 1.23 and nromalize from qt - search the mailinglist) before creating this patch?
I hope you don't find this too restraining, as I've said the patch is really nice we just need to it to match the current style. I'm just buying some time with this to get a chance to test it better ;).
</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Regarding astyle-kmymoney.sh - looks like you're really buying yourself some time ;) since neither normalize nor astyle 1.23 come easy to get.
I ran astyle 2.01 instead, configured like in the astyle-kmymoney.sh. Will that suffice?</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 26th, 2012, 4:56 p.m., <b>Cristian Oneț</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/107137/diff/1/?file=92953#file92953line27" style="color: black; font-weight: bold; text-decoration: underline;">kmymoney/converter/mymoneystatementreader.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">27</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cp">// ----------------------------------------------------------------------------</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Why were these header group markers removed?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I've seen KMM files without the markers, so I assumed these are optional. That's why I followed my personal opinion on this - not to use such code beautifiers.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 26th, 2012, 4:56 p.m., <b>Cristian Oneț</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/107137/diff/1/?file=92953#file92953line45" style="color: black; font-weight: bold; text-decoration: underline;">kmymoney/converter/mymoneystatementreader.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">36</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cp">#include <transactionmatcher.h></span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Project includes should use quotes. I know that it works either way but it's nicer that way.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Ok, I'll fix it for all KMM includes from the patch.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 26th, 2012, 4:56 p.m., <b>Cristian Oneț</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/107137/diff/1/?file=92954#file92954line64" style="color: black; font-weight: bold; text-decoration: underline;">kmymoney/converter/mymoneystatementreader.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">64</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cp">#include <existingtransactionmatchfinder.h></span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Same as above.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Will do.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 26th, 2012, 4:56 p.m., <b>Cristian Oneț</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/107137/diff/1/?file=92956#file92956line23" style="color: black; font-weight: bold; text-decoration: underline;">kmymoney/dialogs/existingtransactionmatchfinder.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">23</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cp">#include <QPair></span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Try to use the same style for new headers as the old one if possible. The KDE 'Krazy' Checker can help check if the headers have the proper license.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">You mean file headers, not the "#include <QPair>", right? Ok, I can fix that.</pre>
<br />
<p>- Łukasz</p>
<br />
<p>On October 30th, 2012, 5:42 p.m., Łukasz Maszczyński wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KMymoney.</div>
<div>By Łukasz Maszczyński.</div>
<p style="color: grey;"><i>Updated Oct. 30, 2012, 5:42 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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")
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">make test</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kmymoney/dialogs/transactionmatchfinder.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>kmymoney/dialogs/transactionmatcher.cpp <span style="color: grey">(c380877)</span></li>
<li>kmymoney/dialogs/transactionmatchfinder.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>CMakeLists.txt <span style="color: grey">(93af070)</span></li>
<li>kmymoney/converter/mymoneystatementreader.h <span style="color: grey">(758ff00)</span></li>
<li>kmymoney/converter/mymoneystatementreader.cpp <span style="color: grey">(42c4841)</span></li>
<li>kmymoney/dialogs/CMakeLists.txt <span style="color: grey">(9a8d782)</span></li>
<li>kmymoney/dialogs/existingtransactionmatchfinder.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>kmymoney/dialogs/existingtransactionmatchfinder.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>kmymoney/dialogs/scheduledtransactionmatchfinder.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>kmymoney/dialogs/scheduledtransactionmatchfinder.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>kmymoney/dialogs/transactionmatcher.h <span style="color: grey">(d09a4cd)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/107137/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>