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

Deal with decimals using PreciseMoney #7

Closed
mathiasverraes opened this issue Dec 8, 2011 · 18 comments · Fixed by #634 · May be fixed by #335
Closed

Deal with decimals using PreciseMoney #7

mathiasverraes opened this issue Dec 8, 2011 · 18 comments · Fixed by #634 · May be fixed by #335

Comments

@mathiasverraes
Copy link
Collaborator

idea from http://joda-money.sourceforge.net/userguide.html

There are two main classes which are both based on BigDecimal:
Money - an amount in a single currency where the decimal places is determined by the currency
BigMoney - an amount in a single currency where there is no restriction on the scale

For example, the currencies of US dollars, Euros and British Pounds all use 2 decimal places, thus a Money instance for any of these currencies will have 2 decimal places. By contrast, the Japanese Yen has 0 decimal places, and thus any Money instance with that currency will have 0 decimal places.

Instances of BigMoney may have any scale that can be supported by BigDecimal.
Conversion between a Money and a BigMoney can be performed using the methods toBigMoney() and toMoney(). The latter optionally takes a rounding mode to handle the case where information must be lost.

@mathiasverraes
Copy link
Collaborator Author

using bcmath?

@sa-tasche
Copy link

using gmp?

@mathiasverraes mathiasverraes added this to the 3.1.0 release milestone Mar 22, 2014
@camspiers
Copy link
Contributor

Related to this, there is a fairly big issue with integer overflow in this library. Although I really like your library Mathias, I choose to use https://github.com/sebastianbergmann/money for various reasons, but I still want to help this library.

If you have a look at my recent pull requests:

This library suffers from the same issues, either silently (and wildly) losing imprecision, or actually failing but not really given appropriate errors.

Regards

@mathiasverraes
Copy link
Collaborator Author

@josecelano
Copy link

Which BigDecimal class are you planning to use?
I am working on an implementation using https://github.com/Litipk/php-bignumbers

@josecelano
Copy link

@mathiasverraes I have done a test implementing Money class using a BigDecimal library (https://github.com/Litipk/php-bignumbers) I have only changed amount type from integer to Decimal.

Here is the test:
https://github.com/josecelano/money/blob/master/lib/Money/BigMoney.php

I call it BigMoney but I only want to use your libary with big integers (I do not need to scale over the currency default decimal places). Then I suppose it is a new implementation of Money. Althougth I think it could be easily changed to implement your BigMoney.

Just after the commit I have found this new library: https://github.com/keiosweb I suppose I can use it for all possible values (int, big int and more presicion). Do you think it's better to have only one class rather than 2 as you propose? I think it is a good idea to have two but the first one (Money) should support big amounts. I suppose there could be countries with hyperinflation where you can easily overflow max integer in 32 bits systems.

@sagikazarmark
Copy link
Collaborator

@josecelano big integer values are supported now on the nextrelease branch.

@josecelano
Copy link

OK thanks @sagikazarmark but I do not see any big integer in that brancg. It seems internal amount attribute is still an integer.

@sagikazarmark
Copy link
Collaborator

Integer support is implemented as Calculators. If you have gmp or bcmath installed, it will automatically use those which allows you to represent big integers.

@josecelano
Copy link

@sagikazarmark I have not had time to test this new verion but there is something I still do not understand, you are using big integer functions for calculations but you are still using an integer to store the amount attribute, then you still have the integer max size limit. For example if I want to add two Money objects and the result is greater than max integer value (in 32 or 64 bit systems), then the result does not fit in an integer.

GMP calculator add method returns a string (gmp_strval) but Money constructor expects an integer.

@sagikazarmark
Copy link
Collaborator

@josecelano that's what the docblock says, but look at the validation:

https://github.com/mathiasverraes/money/blob/nextrelease/src/Money.php#L57

There is a PR which fixes that docblock as well.

@frederikbosch
Copy link
Member

@josecelano This is fixed in #133.

@TNAJanssen
Copy link

What is the status of this issue?

@frederikbosch
Copy link
Member

@TNAJanssen To be implemented after 3.0 will be released.

@frederikbosch
Copy link
Member

@TNAJanssen The initial PR has been proposed.

@frederikbosch
Copy link
Member

@TNAJanssen Maybe you could give some feedback.

@bmeynell
Copy link

bmeynell commented Aug 18, 2017

👍 Agree this is a critical feature and a must-have when requiring handling of fractions of lowest denominations (e.g., "fraction of cents"). Comes up a lot in computing averages, applying discounts, and even simple addition of multiple amounts that are in "fractions of cents".

@frederikbosch frederikbosch changed the title deal with decimals using BigMoney Deal with decimals using PreciseMoney Aug 21, 2017
@frederikbosch
Copy link
Member

@bmeynell If it is critical you might as well help to get it done, e.g. give your feedback on PR #335. It is all too easy to say something is critical and do nothing at all. Nonetheless, your comment helped me awaking the PR, see my comment from today in the thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants