<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/D23813">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/D23813#inline-135096">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ahmadsamir</span> wrote in <span style="color: #4b4d51; font-weight: bold;">kkeysequencewidget.cpp:127</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">I understand. but I was talking about this case specifically, it's about foreach two arguments, and making them const as needed, so that seqAsString change is in inline with replacing foreach with range-for.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Is it?<br />
This patch as is now has 3 changes:</p>

<ul class="remarkup-list">
<li class="remarkup-list-item">change a foreach loop over extracted key list of a QHash to then also access values (2 discouraged things) to iterator-based for loop over the QHash</li>
<li class="remarkup-list-item">change a foreach loop over a QList to a range-based for loop over the QList</li>
<li class="remarkup-list-item">cache the result of toString() outside of the inner loop</li>
</ul>

<p style="padding: 0; margin: 8px;">Each of those is an independent change. The first two both fall into the domain of "port away from foreach". The last one, as could be also seen in the first version of this patch, has nothing to do with them, <tt style="background: #ebebeb; font-size: 13px;">seq</tt> was a const reference before and has been after (well, then represented/replaced by`it.key()`).</p>

<p style="padding: 0; margin: 8px;">Where would you see that "making them const as needed" that you based your reply on? :)</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R263 KXmlGui</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D23813">https://phabricator.kde.org/D23813</a></div></div><br /><div><strong>To: </strong>kossebau, dfaure<br /><strong>Cc: </strong>ahmadsamir, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns<br /></div>