Changeset View
Changeset View
Standalone View
Standalone View
src/core/kfileitem.cpp
Show First 20 Lines • Show All 1232 Lines • ▼ Show 20 Line(s) | 1226 | { | |||
---|---|---|---|---|---|
1233 | } | 1233 | } | ||
1234 | 1234 | | |||
1235 | return d->m_url == other.d->m_url; | 1235 | return d->m_url == other.d->m_url; | ||
1236 | } | 1236 | } | ||
1237 | 1237 | | |||
1238 | bool KFileItem::operator!=(const KFileItem &other) const | 1238 | bool KFileItem::operator!=(const KFileItem &other) const | ||
1239 | { | 1239 | { | ||
1240 | return !operator==(other); | 1240 | return !operator==(other); | ||
1241 | } | 1241 | } | ||
dfaure: This isn't symmetric. operator< must have the property that a<b and b<a aren't both true, and… | |||||
this has to be otherwise (nullptr) < (nullptr). bruns: > If we decide that a null item is inferior to anything else, then we need
> if (!d) return… | |||||
1242 | 1242 | | |||
1243 | bool KFileItem::operator<(const KFileItem &other) const | ||||
1244 | { | ||||
1245 | if (!other.d) { | ||||
1246 | return false; | ||||
1247 | } | ||||
1248 | if (!d) { | ||||
1249 | return other.d->m_url.isValid(); | ||||
1250 | } | ||||
1251 | return d->m_url < other.d->m_url; | ||||
1252 | } | ||||
1253 | | ||||
1254 | bool KFileItem::operator<(const QUrl &other) const | ||||
1255 | { | ||||
1256 | if (!d) { | ||||
1257 | return other.isValid(); | ||||
1258 | } | ||||
1259 | return d->m_url < other; | ||||
1260 | } | ||||
1261 | | ||||
1243 | KFileItem::operator QVariant() const | 1262 | KFileItem::operator QVariant() const | ||
1244 | { | 1263 | { | ||
1245 | return qVariantFromValue(*this); | 1264 | return qVariantFromValue(*this); | ||
1246 | } | 1265 | } | ||
1247 | 1266 | | |||
1248 | QString KFileItem::permissionsString() const | 1267 | QString KFileItem::permissionsString() const | ||
1249 | { | 1268 | { | ||
1250 | if (!d) { | 1269 | if (!d) { | ||
▲ Show 20 Lines • Show All 334 Lines • Show Last 20 Lines |
This isn't symmetric. operator< must have the property that a<b and b<a aren't both true, and that they are both false only if the items are equal.
If this is a valid item and other is null, then this code says that *this<other is false and other<*this is false.
Which would let some algorithms deduce that *this == other, which is completely wrong.
If we decide that a null item is inferior to anything else, then we need
(with newlines and braces of course).
This also calls for a corresponding unittest.