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