D23813: Port away from foreach loops over arguments without calls to owner class

[Ahmad Samir]

ahmadsamir added inline comments.

INLINE COMMENTS

> kossebau wrote in kkeysequencewidget.cpp:127
> Is it?
> This patch as is now has 3 changes:
> 
> - 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
> - change a foreach loop over a QList to a range-based for loop over the QList
> - cache the result of toString() outside of the inner loop
> 
> 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, `seq` was a const reference before and has been after (well, then represented/replaced by`it.key()`).
> 
> Where would you see that "making them const as needed" that you based your reply on? :)

I admit "making them const as needed" is badly phrased; I meant whether the container iterated-over is const to begin with, or can be made const in the range-for to avoid a detach/deep-copy ... etc, depending on the code and what it does. The same for the first argument of the range-for loop.

The "addition"/change I was mainly talking about is the micro-optimization of not calling toString() twice; I think such basic coding practice changes are OK in this context.

REPOSITORY
  R263 KXmlGui

REVISION DETAIL
  https://phabricator.kde.org/D23813

To: kossebau, dfaure
Cc: ahmadsamir, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190912/de47e3d6/attachment.html>