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