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