I felt it was missing.
Details
- Reviewers
ngraham dfaure - Group Reviewers
Frameworks - Commits
- R241:f893784ea4f1: Add == and != operators to KIO::UDSEntry
ctest
Diff Detail
- Repository
- R241 KIO
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 16040 Build 16058: arc lint + arc unit
Please move the implementations in the cpp file, otherwise it will be impossible to change/fix the implementation later on in a binary compatible way.
Also:
- both the operators ought to be const, since they do not mutate the object (and otherwise they cannot be used to compare const objects)
- please fix the style in the declarations: const UDSEntry& other -> const UDSEntry &other
please use QCOMPARE/QVERIFY instead of Q_ASSERT in QTest tests
autotests/udsentrytest.cpp | ||
---|---|---|
301 | typo, "additional" |
Looks ok, but I'm just curious about the use case. "I felt it was missing" doesn't sound as strong an argument as "I need this"...
Slaves are supposed to mostly create those, not compare them, and apps are supposed to use KFileItem rather than UDSEntry directly.
The use case I felt I might need it, is when running stats for a file which I already stated earlier and have a KFileItem already.
And checking if the file changed between the two stats.
Without this it would be hard to do.
Using the entry to compare the KFileItem seemed to me the simplest course of action (KFileItem equals compares only file path) but lacked the equal operators.
Note that there is KFileItem::cmp for a more thorough comparison of two fileitems. But it only looks at some UDS fields -- those that matter at the KDirLister level.
Or maybe it shows that UDSEntry::operator== was missing so KFileItem::cmp uses a poor man's version of that ;)
This confirms the need for such an operator== indeed (though I wouldn't change the implementation of KFileItem::cmp until we see a need to do that).
Thanks for the explanation.