D10414: Add move semantics support to KIO::UDSEntry.

David Faure noreply at phabricator.kde.org
Sun Feb 11 20:16:32 UTC 2018


dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  I actually see little point in verifying that *compiler generated* code copies all members correctly. We can trust the compiler :-)
  
  So, to me this looks ok (minus the last two small comments).
  
  If however you were to implement a more complete checking of fields, I would recommend against hashing (how do you debug it when the expected value is wrong?), and to go instead for a string representation (concatenate data using a separator [doesn't matter if the separator is used in one of the values, there's no splitting back needed], QCOMPARE the two strings, then failures *can* be debugged). But as I said, this doesn't seem necessary here.

INLINE COMMENTS

> udsentrytest.cpp:227
> +/**
> + * Test to verify that move semantics work. This is only useful when ran through a profiling or dubugging tool.
> + */

typo: it's debugging, with an 'e'

> udsentrytest.cpp:233
> +    QTemporaryFile file;
> +    QCOMPARE(file.open(), true);
> +    const QByteArray filePath = QFile::encodeName(file.fileName());

That one, however, could be QVERIFY(), that's what it's for ;)

REPOSITORY
  R241 KIO

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

To: markg, dfaure
Cc: apol, #frameworks, michaelh
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180211/348e2af3/attachment.html>


More information about the Kde-frameworks-devel mailing list