KMyMoney/PlayingWithAlkValue: Difference between revisions

From KDE Community Wiki
No edit summary
(updated current status)
 
(8 intermediate revisions by one other user not shown)
Line 1: Line 1:
This page contains some notes about the port of the mymoneymoney class to AlkValue (from alkimia).
This page contains some notes about the port of the mymoneymoney class to AlkValue (from alkimia).
== Requirements ==
To use this patch it is necessary to install:
- [http://gmplib.org/ GMP 5.0.1]
- [http://kde-apps.org/content/show.php/libalkimia?content=137323 libalkimia] -
Initial Slackbuild script available on request - (kaduardo at gmail dot com)
== Current status ==
- Patch on reviewboard (against r1222953).
Patch complete, the plan is to incorporate it into svn trunk on Friday, 04th Mar 2011.
- Need to update the list of affected classes
Patch has been included in svn trunk on Friday, March 4th 2011.  The transition to AlkValue is now finished.
== Development history ==
Some notes created during development (a kind of timeline), including problems identified during the migration, and the fixes for these problems.
==== Initial strategy ====


Based on the forum, the general strategy is to derive MyMoneyMoney from AlkValue, such that
Based on the forum, the general strategy is to derive MyMoneyMoney from AlkValue, such that
Line 7: Line 30:
In this way, most of the methods should be directly available, and others can be provided by MyMoneyMoney (such as the toDouble() method that is used by several classes).
In this way, most of the methods should be directly available, and others can be provided by MyMoneyMoney (such as the toDouble() method that is used by several classes).


This needs to be checked in order to avoid possible overflows.
- Problem: It is necessary to check some conversion methods (toDouble) in order to avoid other possible overflows.


'''Current status'''
Identify all classes that include mymoneymoney.h. These classes need to be reviewed after the patch is ready.


==== Removing internal variables ====
* Removal of m_num and m_denom from MyMoneyMoney class. Using inheritance from AlkValue. Class compiling ok. Starting unit testing the new class.
* Removal of m_num and m_denom from MyMoneyMoney class. Using inheritance from AlkValue. Class compiling ok. Starting unit testing the new class.


==== String based constructor ====
* Dealing with String based constructor and its respective test.
* Dealing with String based constructor and its respective test.


Line 21: Line 46:


''Is it a fix for the AlkValue class?''  
''Is it a fix for the AlkValue class?''  
Final fix: ''Fixed in libalkimia''.


- Problem: string constructor of AlkValue class normalize the fraction, while old constructor does not.  
- Problem: string constructor of AlkValue class normalize the fraction, while old constructor does not.  
Line 28: Line 55:
''
''
For now follow the old constructor (Test passed without normalization in the constructor)
For now follow the old constructor (Test passed without normalization in the constructor)
Final fix: "By design, all values must be always in normalized form."


- Problem: Failed testNegativeStringConstructor. ''Fix for AlkValue class?''
- Problem: Failed testNegativeStringConstructor. ''Fix for AlkValue class?''
Line 33: Line 62:
Fix: Changed the constructor code to consider negative symbols in separate. Either - or ().
Fix: Changed the constructor code to consider negative symbols in separate. Either - or ().


Final fix: "Fixed in libalkimia".
==== Convert method ====
* Dealing with convert method and its respective test
* Dealing with convert method and its respective test


==== Operators ====
* Dealing with operators
* Dealing with operators


- Issue: gmp requires normalized operands, while the operators of the current implementation of mymoneymoney does not normalize the values. This affect the unit tests. Check whether the normalization will be applied to all operators, or only when explicitly requested (reduce method).
- Issue: gmp requires normalized operands, while the operators of the current implementation of mymoneymoney does not normalize the values. This affect the unit tests. Check whether the normalization will be applied to all operators, or only when explicitly requested (reduce method).


Fix: All values normalized. Check/clean code.
==== testFormatMoney ====
* All tests passed apart from testFormatMoney.
* All tests passed apart from testFormatMoney.


Line 48: Line 84:
m1 is printed as 10
m1 is printed as 10


- Need help with the formatMoney method.
"Need help with the formatMoney method".


MyMoneyMoney(100).formatMoney("", 2) is formatted as 100.00
MyMoneyMoney(100).formatMoney("", 2) is formatted as 100.00
Line 56: Line 92:
Confirm if it is the expected behaviour.
Confirm if it is the expected behaviour.


==== formatMoney method ====
* Dealing with the formatMoney method.
* Dealing with the formatMoney method.


- Found the problem. Overflow when extracting int from mpz_class in the convert method.
Fix: Overflow when extracting int from mpz_class in the convert method.


testformatmoney requires a conversion with precision 10, while testconvert only considers precision 2.
testformatmoney requires a conversion with precision 10, while testconvert only considers precision 2.


==== other test cases ====
* starting other tests of the mymoney module.
* starting other tests of the mymoney module.


All passed
All passed


==== unit test from root directory ====
* Starting tests from the root directory.
* Starting tests from the root directory.


All tests passed.
All tests passed.


==== Sample files ====
* Checking the software using some sample files.
* Checking the software using some sample files.


Line 82: Line 122:
Changed the unit test to show this.
Changed the unit test to show this.


==== patch submitted ====
- Code submitted to review board.
- Code submitted to review board.
Start checking which methods are used by the classes affected by the changes.
Start checking which methods are used by the classes affected by the changes.
==== libalkimia changes ====
Updating the patch for latest changes in libalkimia.




'''Classes that include the mymoneymoney.h file'''
==== patch updated ====
* Patch updated in review board (r1213144).
 
- Problem with pivottabletest and querytabletest.
Non-normalized values are being compared against normalized values.
Necessary to identify where the values used by pivottable are being created.
 
==== patch updated (r1222953) ====
* Patch updated in review board (r1222953).
* Problems fixed by Thomas.
 
== Classes that include the mymoneymoney.h file ==
''This needs to be updated to reflect latest changes in the trunk''.


These classes may be directed affected by the changes.
These classes may be directed affected by the changes.


* converter/webpricequote.h
* converter/webpricequote.h
   Simply creates an MyMoneyMoney object based on a String.
   - m_price variable
 
* converter/webpricequote.cpp
  - void WebPriceQuote::slotParseQuote(const QString& _quotedata)
    Possible lost of precision. m_price = pricestr.toDouble();
 
* converter/mymoneyqifprofile.h
  - class MyMoneyMoney ??
 
* converter/mymoneyqifprofile.cpp
* converter/mymoneyqifprofile.cpp
  - const QString MyMoneyQifProfile::value(const QChar& def, const MyMoneyMoney& valuein) const
  - const MyMoneyMoney MyMoneyQifProfile::value(const QChar& def, const QString& valuein) const
* converter/mymoneygncreader.cpp
* converter/mymoneygncreader.cpp
* dialogs/knewequityentrydlg.cpp
* dialogs/knewequityentrydlg.cpp

Latest revision as of 17:53, 29 September 2011

This page contains some notes about the port of the mymoneymoney class to AlkValue (from alkimia).

Requirements

To use this patch it is necessary to install:

- GMP 5.0.1

- libalkimia - Initial Slackbuild script available on request - (kaduardo at gmail dot com)

Current status

- Patch on reviewboard (against r1222953).

Patch complete, the plan is to incorporate it into svn trunk on Friday, 04th Mar 2011.

- Need to update the list of affected classes

Patch has been included in svn trunk on Friday, March 4th 2011. The transition to AlkValue is now finished.

Development history

Some notes created during development (a kind of timeline), including problems identified during the migration, and the fixes for these problems.

Initial strategy

Based on the forum, the general strategy is to derive MyMoneyMoney from AlkValue, such that

class MyMoneyMoney : public AlkValue {...};

In this way, most of the methods should be directly available, and others can be provided by MyMoneyMoney (such as the toDouble() method that is used by several classes).

- Problem: It is necessary to check some conversion methods (toDouble) in order to avoid other possible overflows.

Identify all classes that include mymoneymoney.h. These classes need to be reviewed after the patch is ready.

Removing internal variables

  • Removal of m_num and m_denom from MyMoneyMoney class. Using inheritance from AlkValue. Class compiling ok. Starting unit testing the new class.

String based constructor

  • Dealing with String based constructor and its respective test.

- Problem: Apparently, AlkValue String constructor can not deal with "1,123.". It seems that the problem is caused by the "," in the converted string, which is not supported by the GMP mpq_class class.

Temporary fix: Copied the AlkValue string constructor to MyMoneyMoney class and eliminated the thousand separator.

Is it a fix for the AlkValue class?

Final fix: Fixed in libalkimia.

- Problem: string constructor of AlkValue class normalize the fraction, while old constructor does not. This caused failures in the testStringConstructor.

Check where the normalization (reduce method) is used on the code. Decide whether normalize in the constructor like the AlkValue or to leave like the old constructor. For now follow the old constructor (Test passed without normalization in the constructor)

Final fix: "By design, all values must be always in normalized form."

- Problem: Failed testNegativeStringConstructor. Fix for AlkValue class?

Fix: Changed the constructor code to consider negative symbols in separate. Either - or ().

Final fix: "Fixed in libalkimia".

Convert method

  • Dealing with convert method and its respective test

Operators

  • Dealing with operators

- Issue: gmp requires normalized operands, while the operators of the current implementation of mymoneymoney does not normalize the values. This affect the unit tests. Check whether the normalization will be applied to all operators, or only when explicitly requested (reduce method).

Fix: All values normalized. Check/clean code.

testFormatMoney

  • All tests passed apart from testFormatMoney.

- Possible problem in the multiplication against int.

MyMoneyMoney m1(100,1); m1 = m1 * 10;

m1 is printed as 10

"Need help with the formatMoney method".

MyMoneyMoney(100).formatMoney("", 2) is formatted as 100.00 MyMoneyMoney(1000).formatMoney("", 2) is formatted as 10.00 MyMoneyMoney(100099).formatMoney("", 2) is formatted as 1,000.99

Confirm if it is the expected behaviour.

formatMoney method

  • Dealing with the formatMoney method.

Fix: Overflow when extracting int from mpz_class in the convert method.

testformatmoney requires a conversion with precision 10, while testconvert only considers precision 2.

other test cases

  • starting other tests of the mymoney module.

All passed

unit test from root directory

  • Starting tests from the root directory.

All tests passed.

Sample files

  • Checking the software using some sample files.

- Crashing when opening the ledger view

Problem identified. Continuing with tests using sample files.

  • Crash when entering new deposit.

- Problem identified. The string constructor is not working for 0.08 or 0.09, although it works for 0.07. Changed the unit test to show this.

patch submitted

- Code submitted to review board. Start checking which methods are used by the classes affected by the changes.

libalkimia changes

Updating the patch for latest changes in libalkimia.


patch updated

  • Patch updated in review board (r1213144).

- Problem with pivottabletest and querytabletest. Non-normalized values are being compared against normalized values. Necessary to identify where the values used by pivottable are being created.

patch updated (r1222953)

  • Patch updated in review board (r1222953).
  • Problems fixed by Thomas.

Classes that include the mymoneymoney.h file

This needs to be updated to reflect latest changes in the trunk.

These classes may be directed affected by the changes.

  • converter/webpricequote.h
 - m_price variable
  • converter/webpricequote.cpp
 - void WebPriceQuote::slotParseQuote(const QString& _quotedata)
   Possible lost of precision. m_price = pricestr.toDouble();
  • converter/mymoneyqifprofile.h
 - class MyMoneyMoney ??
  • converter/mymoneyqifprofile.cpp
 - const QString MyMoneyQifProfile::value(const QChar& def, const MyMoneyMoney& valuein) const
 - const MyMoneyMoney MyMoneyQifProfile::value(const QChar& def, const QString& valuein) const
  • converter/mymoneygncreader.cpp
  • dialogs/knewequityentrydlg.cpp
  • dialogs/knewaccountdlg.h
  • dialogs/ksplittransactiondlg.h
  • mymoney/mymoneybudget.h
  • mymoney/mymoneyforecast.h
  • mymoney/mymoneymoney.cpp
  • mymoney/mymoneysecurity.h
  • mymoney/mymoneyscheduletest.cpp
  • mymoney/mymoneysplit.h
  • mymoney/mymoneytransaction.h
  • mymoney/mymoneystatement.h
  • mymoney/mymoneymoneytest.cpp
  • mymoney/storage/mymoneystoragesql.h
  • mymoney/mymoneyfinancialcalculatortest.cpp
  • mymoney/mymoneyaccount.h
  • mymoney/mymoneyprice.h
  • plugins/printcheck/numbertowords.h
  • reports/reportstestcommon.h
  • views/kscheduletreeitem.cpp
  • widgets/kpricetreeitem.cpp
  • widgets/kmymoneyedit.cpp
  • widgets/registeritem.h
  • wizards/newinvestmentwizard/konlineupdatewizardpage.cpp
  • wizards/newinvestmentwizard/kinvestmentdetailswizardpage.cpp
  • wizards/newuserwizard/knewuserwizard.cpp
  • wizards/newloanwizard/additionalfeeswizardpage.cpp