D16556: Modernize object declarations and initializations
Daniel Vrátil
noreply at phabricator.kde.org
Thu Nov 1 13:10:59 GMT 2018
dvratil requested changes to this revision.
dvratil added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> entitytreemodeltest.cpp:131
> {
> - FakeMonitor *fakeMonitor = new FakeMonitor(this);
> + const auto fakeMonitor = new FakeMonitor(this);
>
<rant>I'm starting to dislike the `QObject`'s "parent" more and more...here `this` becomes the owner of the object, yet semantically the ownership should be in `FakeServerData`. The `const`ness that you added here makes it even more confusing, as it hides the fact that there's ownership transfer...ideally this would be a `std::unique_ptr` that would be `std::move()`d into `FakeServerData`, clearly expressing the ownership and its transfer.</rant>
(You don't have to change this, fixing ownership expression in the codebase is way out-of-scope, especially for tests)
> entitytreemodeltest.cpp:179
> {
> - FakeMonitor *fakeMonitor = new FakeMonitor(this);
> + const auto fakeMonitor = new FakeMonitor{this};
>
You should use the uniform initialization...uniformly :-) So everywhere, or nowhere (except where required to solve most vexing parse - preferred), just don't use it randomly, please.
REPOSITORY
R165 Akonadi
REVISION DETAIL
https://phabricator.kde.org/D16556
To: dkurz, #kde_pim, dvratil
Cc: dvratil, kde-pim, dvasin, rodsevich, winterz, vkrause, mlaurent, knauss
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20181101/1f498f2f/attachment.html>
More information about the kde-pim
mailing list