kossebau added a comment.
In https://phabricator.kde.org/D3987#78427, @kfunk wrote: > In https://phabricator.kde.org/D3987#78420, @kossebau wrote: > > > In https://phabricator.kde.org/D3987#78383, @dfaure wrote: > > > > > I agree. But it's the default value anyway, so why not remove it completely, thus making everyone happy? > > > > > > What do you mean by "remove"? In the samples from a few comments above, the `0` (or now `nullptr`) is used as (default) value for an argument: > > > > + NETWinInfo info(QX11Info::connection(), win, QX11Info::appRootWindow(), nullptr, nullptr); > > + explicit KProgressDialog(QWidget *parent = nullptr, const QString &caption = QString(), > > + const QString &text = QString(), Qt::WindowFlags flags = nullptr); > > + QCOMPARE(QStringList(queryUrl.queryItems(nullptr).keys()).join(", "), > > + KLocale::TimeFormatOptions timeOptions = nullptr, > > > > > > How could that be removed here? Unless you mean replacing with the default constructor of the flags type? That might be even better for human readers at least, agreed :) Though not sure how easy that fix-up is, but maybe the clang artist knows what to do? > > > `Qt::WindowFlags flags = {}`, or > `Qt::WindowFlags flags = Qt::WindowFlags()` "What to do" was about how to create a diff automatically :) Have to say, personally I am used to the `0` to denote a bitmask without any bit set, and would assume quite some other people as well. Not sure if `{}` would be an improvement for the human reader, being another pattern to learn. But assuming we want to avoid `0` as value for QFlags-based types, when comparing NETWinInfo info(QX11Info::connection(), win, QX11Info::appRootWindow(), {}, {}); explicit KProgressDialog(QWidget *parent = nullptr, const QString &caption = QString(), const QString &text = QString(), Qt::WindowFlags flags = {}); QCOMPARE(QStringList(queryUrl.queryItems({}).keys()).join(", "), KLocale::TimeFormatOptions timeOptions = {}, with NETWinInfo info(QX11Info::connection(), win, QX11Info::appRootWindow(), NET::Properties(), NET::Properties2()); explicit KProgressDialog(QWidget *parent = nullptr, const QString &caption = QString(), const QString &text = QString(), Qt::WindowFlags flags = Qt::WindowFlags()); QCOMPARE(QStringList(queryUrl.queryItems(KUrl::QueryItemsOptions()).keys()).join(", "), KLocale::TimeFormatOptions timeOptions = KLocale::TimeFormatOptions(), it might be nice to use `{}` for default values, as the type is given already, and the explicit constructor for arguments when calling methods, as it adds information in the code itself. My 2 cents. In the end I was okay with `0`, just am unhappy with `nullptr` being used for QFlags values, for the reasons given :) REPOSITORY R280 Prison REVISION DETAIL https://phabricator.kde.org/D3987 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kfunk, #frameworks, dfaure, kossebau Cc: kossebau, dfaure, graesslin
