Create d-pointer for LedgerModel
ClosedPublic

Authored by wojnilowicz on Nov 5 2017, 11:44 AM.

Details

Test Plan

Opened new ledger and edited amount of transaction.

Diff Detail

Repository
R261 KMyMoney
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
wojnilowicz created this revision.Nov 5 2017, 11:44 AM
tbaumgart accepted this revision as: tbaumgart.Nov 5 2017, 12:19 PM
tbaumgart added a subscriber: tbaumgart.

I don't see any problem with this. Great work.

kmymoney/models/ledgertransaction.cpp
90

These unused Q_D usages cause lots of compiler warnings with clang. If we want to keep them for sanity, we should comment them out.

This revision is now accepted and ready to land.Nov 5 2017, 12:19 PM

Wow, this is an impressive huge amount of work.

kmymoney/models/ledgermodel.cpp
59

Here I want to throw in my loved qDeleteAll. But do not waste time on it!

kmymoney/models/ledgermodel.h
108

Just for the future I recommend to use std::unique_ptr (from #include <memory>). Qt suggests to prefer the standard library if possible.

wojnilowicz marked an inline comment as done.Nov 5 2017, 12:49 PM

Thank you both for good word :)

kmymoney/models/ledgermodel.cpp
59

It'll be included.

kmymoney/models/ledgermodel.h
108

I'm not against it, although I like Qt syntax better than.
Isn't QScopedPointer used for portability reason?

kmymoney/models/ledgertransaction.cpp
90

In this case a Q_D macro is used. Otherwise I couldn't use d pointer, so I don't understand what do you mean. I don't want to keep them for sanity.

This revision was automatically updated to reflect the committed changes.
wojnilowicz marked an inline comment as done.
tbaumgart added inline comments.Nov 5 2017, 1:18 PM
kmymoney/models/ledgertransaction.cpp
90

Sorry, my mistake. I did not spot the d usage in the next line. I might have been influenced by some changes recently, where I removed a lot of those small functions where the d pointer was not used at all. Sorry for the noise.