D23597: Bulk port away from foreach

[Dominik Haumann]

dhaumann added a comment.


  I think the patch is fine: +1
  
  Please address/comment on `fir` :) besides that, another review won't hurt, since you simplify the code in 1-2 places, i.e. the changes are slightly more than just the transition to `for`.

INLINE COMMENTS

> fonthelpers.cpp:97
>      // Generic fonts, in the inverse of desired order.
> -    QStringList genericNames;
> -    genericNames.append(QStringLiteral("Monospace"));
> -    genericNames.append(QStringLiteral("Serif"));
> -    genericNames.append(QStringLiteral("Sans Serif"));
> +    const QStringList genericNames {
> +        QStringLiteral("Monospace"),

Optionally, you could even write:

  static const auto genericNames = {
        QLatin1String("..."), ...
  };

This initializer list will then even not require any memory allocation. You don't have.contains() later, though... You'd need to simply use std::find() semantics.

> kfontsizeaction.cpp:93
> +        const auto actions = this->actions();
> +        for (QAction* action : actions) {
>              if (action->text() == test) {

Imho here `qAsConst(actions())` would be nicer to avoid this->.

> kviewstateserializer.h:134
>          QStringList itemStrings;
> -        Q_FOREACH(qint64 item, items)
> +        fir (qint64 item : items)
>            itemStrings << QString::number(item);

I think `fir` does not compile, does it?

REPOSITORY
  R236 KWidgetsAddons

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

To: kossebau, #frameworks, cfeck
Cc: dhaumann, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190831/bc6b226a/attachment.html>