D24261: Modernize code: use range-based for loop in more places

[Dominik Haumann]

dhaumann added inline comments.

INLINE COMMENTS

> kedittoolbar.cpp:1566
>  {
> -    XmlDataList::Iterator xit = m_xmlFiles.begin();
> -    for (; xit != m_xmlFiles.end(); ++xit) {
> -        if ((*xit).type() == XmlData::Merged) {
> +    for (auto& xmlFile : m_xmlFiles) {
> +        if (xmlFile.type() == XmlData::Merged) {

No qAsConst? If done by intention, maybe keep the iterator version?

> kgesture.cpp:117
>      d->m_shape.translate(-bounding.left(), -bounding.top());
> -    for (int i = 0; i < d->m_shape.size(); i++) {
> -        d->m_shape[i].setX((int)(xScale * (float)d->m_shape[i].x()));
> -        d->m_shape[i].setY((int)(yScale * (float)d->m_shape[i].y()));
> +    for (QPoint& shape : d->m_shape) {
> +        shape.setX((int)(xScale * (float)shape.x()));

qAsConst?

> kgesture.cpp:131
>      int prevY = d->m_shape[0].y();
> -    for (int i = 1; i < d->m_shape.size(); i++) {
> -        int curX = d->m_shape[i].x();
> -        int curY = d->m_shape[i].y();
> +    for (const QPoint shape : qAsConst(d->m_shape)) {
> +        int curX = shape.x();

1. QPoint &, or is sizeof QPoint==8?
2. Previously, the loop started at index 1.

> kgesture.cpp:165
>  
> -    int i;
> -    for (i = 0; i < d->m_shape.size(); i++) {
> -        ret += QLatin1Char(',') + QString::number(d->m_shape[i].x()) +
> -               QLatin1Char(',') + QString::number(d->m_shape[i].y());
> +    for (const QPoint shape : qAsConst(d->m_shape)) {
> +        ret += QLatin1Char(',') + QString::number(shape.x()) +

QPoint &

> kmainwindow.cpp:349
>      // Clean up for dbus usage: any non-alphanumeric char should be turned into '_'
> -    const int len = dbusName.length();
> -    for (int i = 0; i < len; ++i) {
> -        if (!isValidDBusObjectPathCharacter(dbusName[i])) {
> -            dbusName[i] = QLatin1Char('_');
> +    for (QChar& c : dbusName) {
> +        if (!isValidDBusObjectPathCharacter(c)) {

qAsConst?

> kshortcutseditor.cpp:715
>  
> -    QList<QPair<QString, ColumnDesignation> > shortcutTitleToColumn;
> -    shortcutTitleToColumn << qMakePair(i18n("Main:"), LocalPrimary);
> -    shortcutTitleToColumn << qMakePair(i18n("Alternate:"), LocalAlternate);
> -    shortcutTitleToColumn << qMakePair(i18n("Global:"), GlobalPrimary);
> -    shortcutTitleToColumn << qMakePair(i18n("Global alternate:"), GlobalAlternate);
> +    const QList<QPair<QString, ColumnDesignation>> shortcutTitleToColumnMap {
> +        qMakePair(i18n("Main:"), LocalPrimary),

Maybe make it a QVector. Or simply an initializer list.

> kswitchlanguagedialog_p.cpp:373
>          // We get en-US here but we use en_US
> -        for (int i = 0; i < languagesList.count(); ++i) {
> -            languagesList[i].replace(QLatin1Char('-'), QLatin1Char('_'));
> +        for (auto& language : languagesList) {
> +            language.replace(QLatin1Char('-'), QLatin1Char('_'));

qAsConst

> ktoolbar.cpp:187
>          {
> -            for (int level = 0; level < NSettingLevels; ++level) {
> -                values[level] = Unset;
> +            for (int& value : values) {
> +                value = Unset;

Let's say values detaches here, then the changes value doesn't have any effect on the original one.

> ktoolbar.cpp:376
>              // Scalable icons.
>              const int progression[] = { 16, 22, 32, 48, 64, 96, 128, 192, 256 };
>  

constexpr?

REPOSITORY
  R263 KXmlGui

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

To: kossebau, dfaure
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/20190928/7142824a/attachment-0001.html>