Create d-pointers for MyMoney classes [Part 1]
AbandonedPublic

Authored by wojnilowicz on Oct 30 2017, 5:57 PM.

Details

Reviewers
tbaumgart
Group Reviewers
KMyMoney
Summary

The goal of this patch is to encapsulate classes derived from MyMoneyObject. It follows KDE's Library Code Policy.
It considers all classes derived from MyMoneyObject.

Changes:

  1. d-pointers to move includes for non-PODs to source files, for now they are only for data members,
  2. all accessors in source file and handled through d-pointer, to keep all data members in one place i.e. in PIMPL,
  3. more forward-declarations.

Changes pattern is the same for all classes. Little code changes e.g. some loops use foreach instead of iterators. It avoids crashes.

Test Plan

All tests pass and I couldn't trigger crash during navigation through application.

Diff Detail

Repository
R261 KMyMoney
Lint
Lint Skipped
Unit
Unit Tests Skipped
wojnilowicz created this revision.Oct 30 2017, 5:57 PM
tbaumgart requested changes to this revision.Oct 31 2017, 6:49 PM
tbaumgart added a subscriber: tbaumgart.

I just wanted to test this against current master but it complains about conflicts due to changes I made this morning. Can you please update the patch? Otherwise, it looks good to me, but I wanted to compile and test it a bit. Thanks in advance.

This revision now requires changes to proceed.Oct 31 2017, 6:49 PM
wojnilowicz updated this revision to Diff 21726.Nov 1 2017, 6:35 PM
wojnilowicz edited the summary of this revision. (Show Details)
wojnilowicz edited the test plan for this revision. (Show Details)

Now all classes derived from MyMoneyObject have their own private class.

I just wanted to test this against current master but it complains about conflicts due to changes I made this morning. Can you please update the patch? Otherwise, it looks good to me, but I wanted to compile and test it a bit. Thanks in advance.

No problem. It's good idea to test is thoroughly. I hope you'll find something :)

tbaumgart accepted this revision as: tbaumgart.Nov 2 2017, 3:57 PM

Looks good here as well. Did some testing with online banking without problems.

wojnilowicz abandoned this revision.Nov 3 2017, 3:37 PM