D25677: [KColorScheme/KStatefulBrush] Switch hardcoded numbers for enum items

[David Faure]

dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  So many hardcoded numbers! Much better indeed.

INLINE COMMENTS

> kcolorscheme.cpp:88
>  
> -    _effects[0] = 0;
> -    _effects[1] = 0;
> -    _effects[2] = 0;
> +    for(auto &effect : _effects) effect = 0;
>  

coding style: space after `for`, and add `{` ... `}` around the body,  with newlines.

> kcolorscheme.cpp:393
>  {
> -    switch (role) {
> -    case KColorScheme::AlternateBackground:
> -        return _brushes.bg[1];
> -    case KColorScheme::ActiveBackground:
> -        return _brushes.bg[2];
> -    case KColorScheme::LinkBackground:
> -        return _brushes.bg[3];
> -    case KColorScheme::VisitedBackground:
> -        return _brushes.bg[4];
> -    case KColorScheme::NegativeBackground:
> -        return _brushes.bg[5];
> -    case KColorScheme::NeutralBackground:
> -        return _brushes.bg[6];
> -    case KColorScheme::PositiveBackground:
> -        return _brushes.bg[7];
> -    default:
> -        return _brushes.bg[0];
> +    if(role >= KColorScheme::NormalBackground && role < KColorScheme::NBackgroundRoles) {
> +        return _brushes.bg[role];

coding style: `if (` with a space

(repeats)

> kcolorscheme.cpp:598
>  
> -    for (int i = 0; i < 3; i++) {
> -        QPalette::ColorGroup state = states[i];
> +    for (auto &state : states) {
>          KColorScheme schemeView(state, KColorScheme::View, config);

The `&` seems overkill (and confused me because it looks non-const).
This is just an enum, "copying" by value is faster than following the indirection of a reference.

REPOSITORY
  R265 KConfigWidgets

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

To: ndavis, #frameworks, dfaure
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20191202/51a68d18/attachment-0001.html>