Details
- Reviewers
dvratil - Group Reviewers
KDE PIM - Commits
- R165:562c3904742e: Modernize object declarations and initializations
Diff Detail
- Repository
- R165 Akonadi
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
| autotests/libs/entitytreemodeltest.cpp | ||
|---|---|---|
| 102 | <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 constness 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) | |
| 149 | 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. | |
| autotests/libs/entitytreemodeltest.cpp | ||
|---|---|---|
| 102 | I completely agree with your rant. I read a lot about modern C++ before turning to KDE, and Qt's understanding of ownership prevents you from doing anything that is taught today on that matter, especially about using the type system to express ownership. (Awright, I won't) | |
| 149 | My intent was to replace () with uniform initialization whereever I touch code. If I got you right, you'd prefer the opposite, using parens where possible, so I would now change back all those braces. Feel free to stop me if I misunderstood to spare me some time. | |
| autotests/libs/entitytreemodeltest.cpp | ||
|---|---|---|
| 149 | If you are calling a non-default constructor, you should use (), if you are constructing an object that only has a default ctor and you are initializing members directly, then use {}. | |
| autotests/libs/entitytreemodeltest.cpp | ||
|---|---|---|
| 149 | Wow, I really did miss quite some ctor calls with parens in the first version of this patch. There were more explicit ctor calls with parens than with braces. Anyway, I think I converted them all according to your last comment now. | |