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

ahmadsamir added inline comments. INLINE COMMENTS > kossebau wrote in kkeysequencewidget.cpp:127 > Still unsure what you mean with the constness, as the QHash `shortcuts` has > been const all the time, and thus the access methods and its returned > references. If you meant the initially propos

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

kossebau added inline comments. INLINE COMMENTS > ahmadsamir wrote in kkeysequencewidget.cpp:127 > 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 ..

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

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

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

kossebau added inline comments. INLINE COMMENTS > ahmadsamir wrote in kkeysequencewidget.cpp:127 > 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

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

ahmadsamir added inline comments. INLINE COMMENTS > kossebau wrote in kkeysequencewidget.cpp:127 > As someone who looked at a lot of commit history, my recommendation is: don't > do in the same commit. Make it a separate commit with a dedicated commit > message. > While-at-it changes are annoyi

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

kossebau added inline comments. INLINE COMMENTS > ahmadsamir wrote in kkeysequencewidget.cpp:127 > My two (inexperienced) pennyworth: if it makes sense, and should have been > done to begin with (so most likely it's an oversight), I'd always go for it > (who knows how long it'll be before that

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

ahmadsamir added inline comments. INLINE COMMENTS > kossebau wrote in kkeysequencewidget.cpp:127 > I try (hard, there are many temptations when looking at all exisiting loop > code) to usually do not too much other improvements but stay on-topic of > commit message change, but will do an except

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

kossebau added a comment. Thanks for review :) INLINE COMMENTS > dfaure wrote in kkeysequencewidget.cpp:127 > This could even be `const QString seq = it.key().toString();` so that > toString() is only called once. I try (hard, there are many temptations when looking at all exisiting loop c

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

This revision was automatically updated to reflect the committed changes. Closed by commit R263:02ee352df1af: Port away from foreach loops over arguments without calls to owner class (authored by kossebau). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D23813?vs=65716&id=65808#toc REPOSI

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

dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. (Thread usage is completely unlikely in users of this code, on the containers being passed in, this is really 100% GUI code; it's up to the caller to synchronize this correctly anyway, in t

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

kossebau created this revision. kossebau added a reviewer: dfaure. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. kossebau requested review of this revision. REVISION SUMMARY There is some small risk here: - overseen call chains which still call the own