<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/122138/">https://git.reviewboard.kde.org/r/122138/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 29th, 2015, 11:19 a.m. PST, <b>Christian David</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Sorry for not answering for such a long time! Your work is really good! Do you want to stay? We could need you!</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The queries are more different than I thought and hoped. I hoped to make not one function <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">MyMoneyDbDriver::highestIdNumString(...)</code> but <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">substr(...)</code>, <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">cast(...)</code> etc. But this approach seems to be overly complicated.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The missing tests for different database managers are not so good (btw: correcting some things with the test is another task that could be done ;) ). Maybe I take some time to install a virtual machine with several database managers installed and configured…</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Anyway I think it is ready to be shipped. I would just rename the functions <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">highestIdNumString()</code> to <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">highestNumberFromIdString()</code> (no abbreviations and more precisely - that is the scientist in me ;) ).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The next step is to remove nearly all <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">getRecCount()</code> calls. After the upgrade progress or after creating the database we should init the higest id vars (<code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">m_{institution,account,…}s</code>) <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">once</em> with the new function. Or simply the first time a new id is requested - then we can drop nearly all columns of the table kmmFileInfo - any comments?</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I would like to stay, but unfortunately my studies will require my full-time attention starting next week. I may be able to return once this semester ends. Should I contact you again then?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I had hoped to make the queries more similar as well, but unfortunately not all databases treat text as zero when converting strings to numbers. It may be less computationally expensive to simply retrieve all records from the database with a simple <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">SELECT</code> statement and determine the highest number with a <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">for</code> loop and <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">q.value(0).toString().mid(prefixLength).toULong()</code> (toULong() returns 0 if the conversion fails). It would resolve the test issue as well, since only one query would be necessary for all RDBMSs.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The function is called <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">highestIdNumString()</code> because it returns the string used to get the highest id number from the database, though <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">highestNumberFromIdString()</code> does sound better.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Once again, thank you for all the help, and I hope we can work together again soon!</p></pre>
<br />
<p>- Daniel</p>
<br />
<p>On January 31st, 2015, 1:15 p.m. PST, Daniel Ring wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KMymoney and Christian David.</div>
<div>By Daniel Ring.</div>
<p style="color: grey;"><i>Updated Jan. 31, 2015, 1:15 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=339103">339103</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kmymoney
</div>
<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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This adds a function to retrieve the highest ID number from the database. The IDs are prefixed by a short identifier string, and are stored in the database as strings.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This review request was created as a part of Google Code-In 2014. The specific task it refers to can be found here: https://www.google-melange.com/gci/task/view/google/gci2014/5774859095244800</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I created a small sample database with SQLite and verified that the SQL statment worked properly. I was able to successfully build and run KMyMoney with this function included in Debian Stable (using cmake 2.8.11.1 from backports). However, I was unsure how best to test the function itself, so please let me know if any issues arise.</p></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/mymoney/storage/mymoneydatabasemgrtest.cpp <span style="color: grey">(642fc53d9e220f34e463b2a27ab9a5d4b0c1e3ec)</span></li>
<li>kmymoney/mymoney/storage/mymoneydbdriver.h <span style="color: grey">(e5fe0e9a0058dac1573b6b08087c93c74a06d9a6)</span></li>
<li>kmymoney/mymoney/storage/mymoneydbdriver.cpp <span style="color: grey">(ab7449238bd55e3f4dc543790cd0f38eaa59d787)</span></li>
<li>kmymoney/mymoney/storage/mymoneystoragesql.h <span style="color: grey">(469c0f47d61df783f48787660f526fccdf5264a4)</span></li>
<li>kmymoney/mymoney/storage/mymoneystoragesql.cpp <span style="color: grey">(fa9fbdf7d77eaa4922302d1f583317212af45ddf)</span></li>
<li>kmymoney/mymoney/storage/mymoneydatabasemgrtest.h <span style="color: grey">(07fc175cd9381ef0fc7ad314cf905c22a43fca81)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/122138/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>