Remove MyMoneyObjectContainer
ClosedPublic

Authored by wojnilowicz on Feb 18 2018, 3:21 PM.

Details

Summary

MyMoneyObjectContainer served as double cache presumably for DB storage. For XML storage I see it as redundant, because it has its own cache and after latest changes in KMM it's also redundant for DB storage.

Methods in MyMoneyObjectContainer returned its values as dereferenced pointers thus making it difficult to use compiler optimizations.
Committing transactions to MyMoneyObjectContainer consisted of clearing and refilling whole cache, which isn't very efficient.
Despite observed inefficiencies and removal of MyMoneyObjectContainer, I had not experienced any speed up.

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 requested review of this revision.Feb 18 2018, 3:21 PM
wojnilowicz created this revision.

This diff compiles for me only with the following patch applied

diff --git a/kmymoney/plugins/sql/tests/mymoneydatabasemgr-test.cpp b/kmymoney/plugins/sql/tests/mymoneydatabasemgr-test.cpp
index ef96e24f..4eb24d60 100644
--- a/kmymoney/plugins/sql/tests/mymoneydatabasemgr-test.cpp
+++ b/kmymoney/plugins/sql/tests/mymoneydatabasemgr-test.cpp
@@ -679,7 +679,7 @@ void MyMoneyStorageMgrTest::testReparentAccount()
     ft.commit();
     QVERIFY(ch.value("Key") == "Value");
 
-    MyMoneyFile::instance()->preloadCache();
+    // MyMoneyFile::instance()->preloadCache();
     QVERIFY(m->expense().accountCount() == 3);
     QVERIFY(m->account(ex1.id()).accountCount() == 1);
     QVERIFY(ex3.parentAccountId() == stdAccNames[stdAccExpense]);
@@ -689,7 +689,7 @@ void MyMoneyStorageMgrTest::testReparentAccount()
     m->reparentAccount(ex3, ex1);
     ft.commit();
     //}
-    MyMoneyFile::instance()->preloadCache();
+    // MyMoneyFile::instance()->preloadCache();
     QVERIFY(m->expense().accountCount() == 2);
     QVERIFY(m->account(ex1.id()).accountCount() == 2);
     QVERIFY(ex3.parentAccountId() == ex1.id());
@@ -1211,7 +1211,7 @@ void MyMoneyStorageMgrTest::testRemoveUnusedAccount()
   // now really remove an account
 
   try {
-    MyMoneyFile::instance()->preloadCache();
+    // MyMoneyFile::instance()->preloadCache();
     i = m->institution("I000001");
 
     QVERIFY(i.accountCount() == 0);
@@ -1440,7 +1440,7 @@ void MyMoneyStorageMgrTest::testSetAccountName()
     unexpectedException(e);
   }
 
-  MyMoneyFile::instance()->preloadCache();
+  // MyMoneyFile::instance()->preloadCache();
 
   try {
     QVERIFY(m->liability().name() == "Verbindlichkeiten");

and does not compile with ENABLE_UNFINISHEDFEATURES set to ON. Looks like LedgerModel needs to be changed also.

wojnilowicz updated this revision to Diff 27569.EditedFeb 19 2018, 6:46 PM

Make LedgerModel compile with this patch.

tbaumgart accepted this revision.Feb 20 2018, 7:29 PM

I was now able to build the application with the new ledger code. I ran a few tests which showed no problems. Hopefully, there are no side effects. I would appreciate if you could clean up the commented code before you commit.

kmymoney/mymoney/mymoneyfile.cpp
409

Can you remove unused code before you commit?

This revision is now accepted and ready to land.Feb 20 2018, 7:29 PM
This revision was automatically updated to reflect the committed changes.