Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate to JodaMoney or JavaMoney #5

Open
thomaseizinger opened this issue Dec 28, 2017 · 0 comments
Open

Migrate to JodaMoney or JavaMoney #5

thomaseizinger opened this issue Dec 28, 2017 · 0 comments

Comments

@thomaseizinger
Copy link
Member

thomaseizinger commented Dec 28, 2017

Currently, we have self-made classes for handling amount and currencies. We should replace that with JodaMoney (http://www.joda.org/joda-money/) or JavaMoney (https://github.com/JavaMoney/jsr354-ri)

@thomaseizinger thomaseizinger changed the title Migrate to JodaMoney Migrate to JodaMoney or JavaMoney Dec 28, 2017
thomaseizinger added a commit that referenced this issue Sep 30, 2019
The issue was manifold:

1. The comparison between the computed balances was flawed as we always
compared the two HashMaps directly. Sometimes though, the initial
HashMap for the initial balances contains a balance of 0. This one then
sometimes disappears during the optimization. As a result, the
comparsion sometimes failed even though the actual balances were equal
but just an entry with balance 0 was missing.

2. We used to have two different scales in our Amount class, one for
the internal representation of the BigDecimal and one that we used to
calculate a presentable number to the user. The deemed to be a bad
approach because all kinds of weird behaviour was uncovered by the
property based test. Hence, I removed the internal scale and now simply
drop all the precision after the 2nd decimal.
This is actually drops some precision for currencies that have more than
two decimal places but we don't really actively support those anyway.
In general, we should probably do #5 to properly fix this.
thomaseizinger added a commit that referenced this issue Oct 5, 2019
The issue was manifold:

1. The comparison between the computed balances was flawed as we always
compared the two HashMaps directly. Sometimes though, the initial
HashMap for the initial balances contains a balance of 0. This one then
sometimes disappears during the optimization. As a result, the
comparsion sometimes failed even though the actual balances were equal
but just an entry with balance 0 was missing.

2. We used to have two different scales in our Amount class, one for
the internal representation of the BigDecimal and one that we used to
calculate a presentable number to the user. The deemed to be a bad
approach because all kinds of weird behaviour was uncovered by the
property based test. Hence, I removed the internal scale and now simply
drop all the precision after the 2nd decimal.
This is actually drops some precision for currencies that have more than
two decimal places but we don't really actively support those anyway.
In general, we should probably do #5 to properly fix this.
thomaseizinger added a commit that referenced this issue Oct 5, 2019
The issue was manifold:

1. The comparison between the computed balances was flawed as we always
compared the two HashMaps directly. Sometimes though, the initial
HashMap for the initial balances contains a balance of 0. This one then
sometimes disappears during the optimization. As a result, the
comparsion sometimes failed even though the actual balances were equal
but just an entry with balance 0 was missing.

2. We used to have two different scales in our Amount class, one for
the internal representation of the BigDecimal and one that we used to
calculate a presentable number to the user. The deemed to be a bad
approach because all kinds of weird behaviour was uncovered by the
property based test. Hence, I removed the internal scale and now simply
drop all the precision after the 2nd decimal.
This is actually drops some precision for currencies that have more than
two decimal places but we don't really actively support those anyway.
In general, we should probably do #5 to properly fix this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant