Use range loop if possible
ClosedPublic

Authored by gawin on Aug 7 2019, 7:46 PM.

Details

Summary

It (should) simplify code a bit. It some cases prevents copying strings, shared_ptrs.

Diff Detail

Repository
R40 Kate
Lint
Lint Skipped
Unit
Unit Tests Skipped
gawin created this revision.Aug 7 2019, 7:46 PM
Restricted Application added a subscriber: kwrite-devel. · View Herald TranscriptAug 7 2019, 7:46 PM
gawin requested review of this revision.Aug 7 2019, 7:46 PM
gawin added inline comments.Aug 7 2019, 7:50 PM
addons/filebrowser/katefilebrowserconfig.cpp
162

I'm not sure about name. Maybe actionName would be better?

kate/katemdi.cpp
983–984

Also, camel case?

cullmann requested changes to this revision.Aug 7 2019, 7:51 PM
cullmann added a subscriber: cullmann.

I think if we touch all this, we should use in some places qAsConst (or const ... local variable) to avoid detach, see more detailed explanation here:

https://www.kdab.com/goodbye-q_foreach/

(that howto is for a different kind of conversion but the hints for qAsConst are ok for this, too)

e.g.

QStringList children = config.readEntry("Children", QStringList());
for (const auto& str : children) {

>

const QStringList children = config.readEntry("Children", QStringList());
for (const auto& str : children) {

or

for (auto& pluginInfo : m_pluginList) {

>

for (auto& pluginInfo : qAsConst(m_pluginList)) {

This revision now requires changes to proceed.Aug 7 2019, 7:51 PM
cullmann added inline comments.Aug 7 2019, 7:54 PM
addons/filebrowser/katefilebrowserconfig.cpp
162

Why not ;=)

kate/katemdi.cpp
983–984

If you like, have no issues with both variants. But better qAsConst(m_toolViews)

gawin added a comment.EditedAug 7 2019, 9:11 PM

qAsConst has problems if we want to take addr to object:

for (auto& pluginInfo : qAsConst(m_pluginList)) {
    m_name2Plugin[pluginInfo.saveName()] = &pluginInfo;
}

&pluginInfo will be const ptr.

gawin updated this revision to Diff 63349.Aug 8 2019, 11:28 AM

Fixes to avoid detach (if possible).

cullmann accepted this revision.Aug 8 2019, 11:55 AM

Ok, looks good to me.
Thanks for taking care.

This revision is now accepted and ready to land.Aug 8 2019, 11:55 AM
This revision was automatically updated to reflect the committed changes.

Hmpf :/ I landed the patch via arc land, seems it used me as author :/

gawin added a comment.Aug 8 2019, 12:11 PM

It's ok, unless you don't "stand behind" this code. ;)

I actually prefer to keep the author intact to attribute the work properly.

I mean: you did contribute, you shall be mentioned in the log :/

Anyways, thanks for the improvements, the new range based loops are a good thing to use.