D15946: Avoid creation of needless temporary containers
Kai Uwe Broulik
noreply at phabricator.kde.org
Thu Oct 4 15:37:04 BST 2018
broulik added a comment.
Nice findings, feel free to ignore the stylistic changes I commented, except the `qDeleteAll` one, and do unrelated further cleanup in a separate patch
INLINE COMMENTS
> waylandoutput.cpp:64
> + auto it = std::find(m_modeIdMap.cbegin(), m_modeIdMap.cend(), kwaylandmodeid);
> + if (it == m_modeIdMap.cend()) {
> qCWarning(KSCREEN_WAYLAND) << "Invalid kwayland mode id:" << kwaylandmodeid << m_modeIdMap;
I think we typically use `const...()` instead of `c...()` but since this method is `const`, shouldn't be neccessary to begin with
> qscreenconfig.cpp:47
> {
> - foreach(auto output, m_outputMap.values()) {
> + foreach(auto output, m_outputMap) {
> delete output;
`qDeleteAll(m_outputMap);`
> qscreenconfig.cpp:62
> {
> QList<int> ids;
> + foreach(auto output, m_outputMap) {
This seems unused
> output.cpp:101
> {
> - if (before.keys() != after.keys()) {
> + if (before.size() != after.size()) {
> return false;
`count()`
> waylandconfigreader.cpp:117
> + if (mode.contains(QStringLiteral("refreshRate"))) {
> m0.refreshRate = qRound(mode[QStringLiteral("refreshRate")].toReal() * 1000); // config has it in Hz
> }
We do a double lookup here, `contains()` and then `operator[]` afterwards, should be combined to a single `find()`
REPOSITORY
R110 KScreen Library
REVISION DETAIL
https://phabricator.kde.org/D15946
To: volkov, #plasma
Cc: broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20181004/a548af2e/attachment.html>
More information about the Plasma-devel
mailing list