Replace usage of SlaveBase::config() by SlaveBase::mapConfig()
ClosedPublic

Authored by meven on Oct 12 2019, 7:48 AM.

Details

Summary

Depends on D23523

Now that SlaveBase::config() is marked deprecated use the new function SlaveBase::mapConfig() instead preventing KConfig I/O when reading config

Diff Detail

Repository
R241 KIO
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
meven created this revision.Oct 12 2019, 7:48 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptOct 12 2019, 7:48 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
meven requested review of this revision.Oct 12 2019, 7:48 AM
meven planned changes to this revision.Oct 12 2019, 8:00 AM
meven updated this revision to Diff 67767.Oct 12 2019, 8:19 AM

Replace more config() calls, fixes, code cleanup

meven updated this revision to Diff 67768.Oct 12 2019, 8:20 AM

Replace more config() calls, fixes, code cleanup

meven updated this revision to Diff 67772.Oct 12 2019, 8:29 AM

Clean up a QVariant()

Looks fine, but a bit verbose. Maybe we could have configValue() overloads (in SlaveBase) for bool, int and QString, to cover the most common use cases?

- mapConfig().value(QStringLiteral("MaxCacheAge"), DEFAULT_MAX_CACHE_AGE).toInt();
+ configValue(QStringLiteral("MaxCacheAge"), DEFAULT_MAX_CACHE_AGE);

Given how many calls there are, I would think it's worth it.

(for other more unusual cases like QStringList the current solution remains)

Also, if one day in the far future we want to replace QMap with std::map or other, all these calls to value() will be a PITA, while SlaveBase::configValue() would be trivial to port :)

meven updated this revision to Diff 67943.Oct 15 2019, 8:22 AM

Add configValue overloads to simplify code

meven added a comment.Oct 15 2019, 8:25 AM

Looks fine, but a bit verbose. Maybe we could have configValue() overloads (in SlaveBase) for bool, int and QString, to cover the most common use cases?

- mapConfig().value(QStringLiteral("MaxCacheAge"), DEFAULT_MAX_CACHE_AGE).toInt();
+ configValue(QStringLiteral("MaxCacheAge"), DEFAULT_MAX_CACHE_AGE);

Given how many calls there are, I would think it's worth it.

(for other more unusual cases like QStringList the current solution remains)

Good suggestion, I was thinking about this myself.
Done

Also, if one day in the far future we want to replace QMap with std::map or other, all these calls to value() will be a PITA, while SlaveBase::configValue() would be trivial to port :)

:)

There are still a couple of more complicated use of config() to deal with

dfaure accepted this revision.Oct 16 2019, 6:45 AM
This revision is now accepted and ready to land.Oct 16 2019, 6:45 AM
This revision was automatically updated to reflect the committed changes.