graesslin added inline comments.

INLINE COMMENTS

> xrandrconfig.cpp:517-520
> +    int modeId = kscreenOutput->currentModeId().toInt();
> +    if (!kscreenOutput->currentMode()) {
> +        modeId = kscreenOutput->preferredModeId().toInt();
> +    }

it looks strange to me that you first access the currentModeId and afterwards 
check whether there is a currentMode.

What about a one liner?

  const int modeId = kscreenOutput->currentMode() ? 
kscreenOutput->currentModeId().toInt() : 
kscreenOutput->preferredModeId().toInt();

If you don't like the one-liner (which I can understand) I would inverse the 
logic and init the modeId with preferredModeId and only set to currentModeId if 
there is a currentMode.

> xrandrconfig.cpp:559-562
> +    int modeId = kscreenOutput->currentModeId().toInt();
> +    if (!kscreenOutput->currentMode()) {
> +        modeId = kscreenOutput->preferredModeId().toInt();
> +    }

As you have twice the same code:

  static inline int getBestModeId(auto *kscreenOutput) {
      return ....
  }

That could also address my other comment nicely as then you can do nice 
if-else-return switches.

REPOSITORY
  rLIBKSCREEN KScreen Library

REVISION DETAIL
  https://phabricator.kde.org/D2330

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: sebas, dvratil
Cc: graesslin, plasma-devel, #plasma, ali-mohamed, jensreuterberg, abetts, sebas
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to