<table><tr><td style="">kossebau added inline comments.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D25039">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D25039#inline-141630">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.cpp:1376</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "> <span class="n">QString</span> <span class="n">parentDir</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="n">QString</span> <span class="n">filename</span> <span style="color: #aa2211">=</span> <span class="n">tempurl</span><span class="p">.</span><span class="n">fileName</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="bright"></span><span style="color: #aa4000"><span class="bright">const</span></span><span class="bright"> </span><span class="n">QString</span> <span class="bright"></span><span style="color: #aa2211"><span class="bright">&</span></span><span class="n">filename</span> <span style="color: #aa2211">=</span> <span class="n">tempurl</span><span class="p">.</span><span class="n">fileName</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; "> <span class="n">Q_ASSERT</span><span class="p">(</span><span style="color: #aa2211">!</span><span class="n">filename</span><span class="p">.</span><span class="n">isEmpty</span><span class="p">());</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;"><tt style="background: #ebebeb; font-size: 13px;">QUrl::fileName()</tt> returns a value QString, so just</p>
<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">const QString filename = empurl.fileName();</pre></div>
<p style="padding: 0; margin: 8px;">While</p>
<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">const QString &filename = empurl.fileName();</pre></div>
<p style="padding: 0; margin: 8px;">also is fine code IIRC, as I once learned to my surprise, as the <tt style="background: #ebebeb; font-size: 13px;">const QString &</tt> here means to tell the compiler the actual value instance should be hold only until the last use of the variable, not the end of the scope in which the variable exists, this seems also not well known by others, so might only make them confuse.<br />
Given this is an optimization not needed here, IMHO no need to use this technique here.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D25039#inline-141560">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kossebau</span> wrote in <span style="color: #4b4d51; font-weight: bold;">kcookiejar.cpp:1103</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">This here seems some overleft from when the method actually needed a modfyable copy of _domain. Seems this in no longer the case.<br />
So we can just rename <tt style="background: #ebebeb; font-size: 13px;">_domain</tt> to <tt style="background: #ebebeb; font-size: 13px;">domain</tt> in the arg list instead, and be done.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Hm, why the change of all _domain to domain, not the other way around?<br />
The _xyz naming of argument variables has been seen usually when it was needed to create a copy of the argument in the implementation, because e.g. the const-refness was in the way. As the normal naming of arguments and variables is without the _ prefix,. The prefix is then used with the argument name to denote this is the input to the actual variable then used in the implementation.</p>
<p style="padding: 0; margin: 8px;">So:</p>
<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">void foo(const Type arg)
{
// need to do non-const things with arg value, meh
}</pre></div>
<p style="padding: 0; margin: 8px;">-></p>
<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">void foo(const Type _arg)
{
Type arg(_arg);
// do non-const things with arg value
}</pre></div></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D25039#inline-141635">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kdirmodel.cpp:495</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "> <span style="color: #aa4000">if</span> <span class="p">(</span><span style="color: #aa2211">!</span><span class="n">urlsBeingFetched</span><span class="p">.</span><span class="n">isEmpty</span><span class="p">())</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">const</span> <span class="n">QUrl</span> <span class="n">dirUrl</span><span class="p">(</span><span class="n">url</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">const</span> <span class="n">QUrl</span> <span class="bright"></span><span style="color: #aa2211"><span class="bright">&</span></span><span class="n">dirUrl</span><span class="p">(</span><span class="n">url</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; "> <span style="color: #aa4000">for</span> <span class="p">(</span><span style="color: #aa4000">const</span> <span class="n">QUrl</span> <span style="color: #aa2211">&</span><span style="color: #a0a000">urlFetched</span> <span class="p">:</span> <span class="n">qAsConst</span><span class="p">(</span><span class="n">urlsBeingFetched</span><span class="p">))</span> <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;"><tt style="background: #ebebeb; font-size: 13px;">const QUrl &dirUrl = url;</tt> for consistency, please.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R241 KIO</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D25039">https://phabricator.kde.org/D25039</a></div></div><br /><div><strong>To: </strong>meven, Frameworks, dfaure<br /><strong>Cc: </strong>anthonyfieroni, kossebau, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns<br /></div>