<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/113188/">http://git.reviewboard.kde.org/r/113188/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 10th, 2013, 10:32 a.m. UTC, <b>David Edmundson</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/113188/diff/1/?file=200242#file200242line139" style="color: black; font-weight: bold; text-decoration: underline;">services/storage/resourceidentifier.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; ">bool Nepomuk2::ResourceIdentifier::runIdentification(const KUrl& uri)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">126</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QUrl</span> <span class="n">nieUrl</span> <span class="o">=</span> <span class="n">res</span><span class="p">.</span><span class="n">nieUrl</span><span class="p">();</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">139</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QUrl</span> <span class="n">nieUrl</span> <span class="o">=</span> <span class="n">res</span><span class="p">.</span><span class="n">nieUrl</span><span class="p">();</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;">could be const&
same for newUri, and the ids later.</pre>
</blockquote>
<p>On October 10th, 2013, 6:05 p.m. UTC, <b>Vishesh Handa</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;">Are you sure? The nieUrl method returns a temporarily constructed QUrl. Trying to create a reference to a temporary object does not seem like a good idea.</pre>
</blockquote>
<p>On October 10th, 2013, 6:09 p.m. UTC, <b>Vishesh Handa</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;">Okay. I checked and it doesn't seem to make any difference. The constructor is only called once.
Tested by doing -
Class TestClass {
public:
TestClass() {
cout << "Constructor\n";
a = 5;
}
int a;
}
TestClass foo() { TestClass c; c.a = 99; return c; }
int main() {
TestClass cl = foo();
cout << cl.a << endl;
const TestClass& cl2 = foo();
cout << cl2.a << endl;
return 0;
}
The output is "Constructor\n 99\n Contructor\n 99\n". So, meh! Doesn't make any difference. Will investigate more.
</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;">Some people's rule of thumb is "if you expect something to be const, add the keyword". It can help, won't do any harm.
You're quite probably right it won't add anything. I didn't mark it as "an issue".
As for why it doesn't explode:
http://herbsutter.com/2008/01/01/gotw-88-a-candidate-for-the-most-important-const/
</pre>
<br />
<p>- David</p>
<br />
<p>On October 10th, 2013, 10:20 a.m. UTC, Vishesh Handa wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Nepomuk and David Edmundson.</div>
<div>By Vishesh Handa.</div>
<p style="color: grey;"><i>Updated Oct. 10, 2013, 10:20 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
nepomuk-core
</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;"> We check if another contact with the same identifier exists during the
identification phase. This way even if the nickname has changed, as long
as the contactUID is the same, a new contact is not created.
</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>services/storage/resourceidentifier.cpp <span style="color: grey">(c59bba355ba7cebfac8d79b642a3486993b58e26)</span></li>
<li>services/storage/test/identificationtests.h <span style="color: grey">(4f2c0280ae4e090dfa307ad83188128ec6c4c267)</span></li>
<li>services/storage/test/identificationtests.cpp <span style="color: grey">(0ed1343a0684418bfa499296f254e273cb8db4fc)</span></li>
<li>services/storage/test/qtest_dms.cpp <span style="color: grey">(d758a81ba66f0a86f12ac2b7a57454ab0bb66575)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/113188/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>