D24262: Modernize code: use range-based loops & algorithms in more places

This revision was automatically updated to reflect the committed changes. Closed by commit R306:65c55de00bae: Modernize code: use range-based loops algorithms in more places (authored by kossebau). REPOSITORY R306 KParts CHANGES SINCE LAST UPDATE

D24262: Modernize code: use range-based loops & algorithms in more places

kossebau added inline comments. INLINE COMMENTS > dfaure wrote in browseropenorsavequestion.cpp:270 > I give C++11/14/17 trainings, if it helps :-) :) Though, I doubt you give them in single person understanding-pace-driven tiredness-cut bed settings (besides own family how to tell a nerd

D24262: Modernize code: use range-based loops & algorithms in more places

dfaure accepted this revision. dfaure added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > kossebau wrote in browseropenorsavequestion.cpp:270 > Sigh... I learned C++ originally from the Stroustrup book, mostly reading in > bed in leisure time. C++11 I

D24262: Modernize code: use range-based loops & algorithms in more places

kossebau added inline comments. INLINE COMMENTS > dfaure wrote in browseropenorsavequestion.cpp:270 > auto or not auto isn't the question when it comes to const :-) > > Would you have written `foreach(QString , apps)` if `apps` was a > qstringlist (to simplify)? > Or `for (QString : apps)` ?

D24262: Modernize code: use range-based loops & algorithms in more places

kossebau updated this revision to Diff 67046. kossebau marked 3 inline comments as done. kossebau added a comment. - move &/* to varname - explicit const with auto types REPOSITORY R306 KParts CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24262?vs=66996=67046 BRANCH

D24262: Modernize code: use range-based loops & algorithms in more places

dfaure added inline comments. INLINE COMMENTS > kossebau wrote in browseropenorsavequestion.cpp:270 > In most projects I have seen people omitting the const, and only adding > explicitly any &/* to help the human reader a bit more. > If you prefer explicit const, fine with me, personally

D24262: Modernize code: use range-based loops & algorithms in more places

kossebau added a comment. In D24262#539348 , @ahmadsamir wrote: > @kossebau: did you try the C++ Standard (working draft): https://isocpp.org/blog/2013/10/n3797-working-draft-standard-for-programming-language-c-stefanus-du-toit > > This ^

D24262: Modernize code: use range-based loops & algorithms in more places

kossebau added inline comments. INLINE COMMENTS > dfaure wrote in browseropenorsavequestion.cpp:270 > Why not *const* auto ? In most projects I have seen people omitting the const, and only adding explicitly any &/* to help the human reader a bit more. If you prefer explicit const, fine with

D24262: Modernize code: use range-based loops & algorithms in more places

dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. Please make the loop variable const-ref whenever possible. INLINE COMMENTS > browseropenorsavequestion.cpp:270 > QObject::connect(menu, ::triggered, d, >

D24262: Modernize code: use range-based loops & algorithms in more places

ahmadsamir added a comment. @kossebau: did you try the C++ Standard (working draft): https://isocpp.org/blog/2013/10/n3797-working-draft-standard-for-programming-language-c-stefanus-du-toit This ^ one is circa 2012. All the sources used to generate the C++ Standard drafts are

D24262: Modernize code: use range-based loops & algorithms in more places

kossebau added a comment. Thanks for your teaching, appreciated :) Will have another look again when not tired. Just tried again to read on cppreference.com the stuff about lambda capturing, but still not digested what I read this afternoon, reread now did not help. So next try scheduled.

D24262: Modernize code: use range-based loops & algorithms in more places

dhaumann added a comment. In D24262#539317 , @dfaure wrote: > It's actually quite clear in my head, because I imagine the generated class. A captured variable in a lambda becomes a member variable. If it's a capture by value (which is what

D24262: Modernize code: use range-based loops & algorithms in more places

dhaumann added inline comments. INLINE COMMENTS > kossebau wrote in plugin.cpp:196 > Confirmed by experiments. Still not yet found a document where explicitly it > is mentioned that the copy constructor will be invoked to generate a copy of > the object for any captured variables only being of

D24262: Modernize code: use range-based loops & algorithms in more places

dfaure added a comment. It's actually quite clear in my head, because I imagine the generated class. A captured variable in a lambda becomes a member variable. If it's a capture by value (which is what happens with [library]), it's a "plain value" member. So: bool

D24262: Modernize code: use range-based loops & algorithms in more places

kossebau added inline comments. INLINE COMMENTS > kossebau wrote in plugin.cpp:196 > Meh, I need to check quite some code of mine then, I got that wrong and > thought that the values of the actual variables listed are captured (i.e. for > a reference type the reference "pointer"), and not that

D24262: Modernize code: use range-based loops & algorithms in more places

kossebau updated this revision to Diff 66996. kossebau added a comment. properly mark captured variable to be captured by reference as intended REPOSITORY R306 KParts CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24262?vs=66950=66996 BRANCH morerangebased REVISION DETAIL

D24262: Modernize code: use range-based loops & algorithms in more places

kossebau added inline comments. INLINE COMMENTS > dfaure wrote in plugin.cpp:196 > STL is fine by definition, it's the C++ standard. > > But yes, no need for cbegin/cend on a const container. > > Catching `library` by reference makes sense, just like you wouldn't pass it > by value to a

D24262: Modernize code: use range-based loops & algorithms in more places

dfaure added inline comments. INLINE COMMENTS > kossebau wrote in plugin.cpp:196 > Do we have rules in KF whether stl-like names liks cbegin() & cend() are fine? > Then plugins is const, so begin()/end() are giving use the const iterators > already. > > Does it make sense to catch the

D24262: Modernize code: use range-based loops & algorithms in more places

kossebau added inline comments. INLINE COMMENTS > dhaumann wrote in plugin.cpp:196 > cbegin() + cend() > Catch library by reference? [& library] Do we have rules in KF whether stl-like names liks cbegin() & cend() are fine? Then plugins is const, so begin()/end() are giving use the const

D24262: Modernize code: use range-based loops & algorithms in more places

dhaumann added inline comments. INLINE COMMENTS > plugin.cpp:196 > > -QObjectList::ConstIterator it = plugins.begin(); > -for (; it != plugins.end(); ++it) { > -Plugin *plugin = qobject_cast(*it); > -if (plugin && plugin->d->m_library == library) { > -return

D24262: Modernize code: use range-based loops & algorithms in more places

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 GIT_SILENT TEST PLAN Tests pass, KParts-based apps work as before. REPOSITORY