How to deal with integer limits? about money HOT 11 CLOSED

You are right in everything you say. Unrestricted integer support has been discussed before and would be possible with something like bcmath (#7), but it just hasn't been implemented.

I think that we should at least add a clear message in the README that money sizes are limited by integer size in your environment (32bit vs 64bit), and if you are using a database the same holds true for that.

There are also some other things to be aware of, if you look over at the @sebastianbergmann implementation of Money, I added a bunch of failure states when arithmetic on money would cause loss of precision (and sometimes completely crazy values).

https://github.com/sebastianbergmann/money/commits?author=camspiers

These changes should probably be ported over to this library too. These problems become particularly pernicious when dealing in currencies like bitcoin, but they are also a huge problem if people don't validate ecommerce inputs well.

To give an obvious scenario, imagine a product worth $5000, on a 32 bit system, adding 500000 of these items to your cart will cause your cart to have a negative total, WTF, let's just hope that people have their systems setup well to catch this stuff.

https://github.com/mathiasverraes/money/blob/master/lib/Money/Money.php#L216

from money.

Thank you for the great answer! Those check you implemented on Sebastian Bergmann's implementation are indeed pretty great. We ended up rolling our own implementation 100% based on that implementation, minus the currency (we don't have to deal with currencies).

For the database I map PHP's integers to BIGINT. At any point (from DB, to DB, etc.) if there's an int overflow there will be an exception, so at least there won't be any silent data corruption/wtf.

It would indeed be good to implement such checks in this library too.

from money.

You're welcome. I don't have bandwidth at the moment, but I am fairly confident that were you to provide a PR @mathiasverraes would be keen to add the changes. Good luck.

from money.

Recently I saw this being discussed on Twitter too. I believe the overall conclusion is that best thing to use would be GMP, if it is not available on the system switch to BC Math. Other libraries seem to prioritize this way too.

However, there might also be reasons for using BC Math over GMP because GMP only deals with integers, and not with decimals (there is a proposal for the GMP implementation). For another money library this was a reason to choose BC Math over GMP.

Taking into account the feature state of this library, I would say that best thing to prioritize as follows: GMP, BC Math and finally the (current) native PHP method. I am happy to implement and push a PR soon.

from money.

Please do :)
On 25 Nov 2015 10:38, "Frederik Bosch" [email protected] wrote:

Recently I saw this being discussed on Twitter too. I believe the overall
conclusion is that best thing to use would be GMP
http://nl3.php.net/manual/en/book.gmp.php, if it is not available on
the system switch to BC Math http://nl3.php.net/manual/en/book.bc.php.
Other ircmaxell/PHP-PasswordLib#10 libraries
seem to prioritize this way too.

However, there might also be reasons for using BC Math over GMP because
GMP only deals with integers, and not with decimals (there is a proposal
https://wiki.php.net/rfc/gmp-floating-point for the GMP
implementation). For another money library this was a reason to choose BC
Math over GMP https://github.com/ulabox/money.

Taking into account the feature state of this library, I would say that
best thing to prioritize as follows: GMP, BC Math and finally the (current)
native PHP method. I am happy to implement and push a PR soon.

—
Reply to this email directly or view it on GitHub
#111 (comment)
.

from money.

Should we have a separate class for this (like the BigMoney idea)?

from money.

Not in my opinion. I would rather build this in. Why would you want to consume another api for larger values? Who knows when to use BigMoney? It would lead to unexpected results in the Money class, because you would have throw exceptions when you cannot calculate a result because the resulting or initial value is too large. The developer does not know when that is happening in developing time. How would a new BigMoney class interoperate with a Money class?

My idea was to pick a strategy in the constructor. Something like.

$calculators = [new GmpCalculator(), new BcMathCalculator(), new PhpCalculator()];
foreach ($calculators as $calculator) {
  if ($calculator->isSupported()) {

  }
}

from money.

BigMoney is something different: it means money with unlimited division.
You may want to work with something smaller than 1 cent, and round only
when you make an actual payment. That is fundamentally different behaviour
from Money, which always works with the smallest denomination.

That said, I agree the name BigMoney is confusing. I took from the java
implementation but PreciseMoney is probably a better name.

Both Money and PreciseMoney should be implemented with the better math
libs. Please PR against the nextrelease branch.

—
Mathias Verraes
Software Consultant
Value Object Comm.V http://value-object.com
Blog http://verraes.net/ - Email [email protected] - Twitter
http://twitter.com/mathiasverraes - LinkedIn
https://www.linkedin.com/in/mathiasverraes

On 25 November 2015 at 12:43, Frederik Bosch [email protected]
wrote:

Not in my opinion. I would rather build this in. Why would you want to
consume another api for larger values? Who knows when to use BigMoney? It
would lead to unexpected results in the Money class, because you would have
throw exceptions when you cannot calculate a result because the resulting
or initial value is too large. The developer does not know when that is
happening in developing time. How would a new BigMoney class interoperate
with a Money class?

My idea was to pick a strategy in the constructor. Something like.

$calculators = [new GmpCalculator(), new BcMathCalculator(), new PhpCalculator()];foreach ($calculators as $calculator) { if ($calculator->isSupported()) { }}

—
Reply to this email directly or view it on GitHub
#111 (comment)
.

from money.

Just created a PR for this feature.

from money.

This one can be closed.

from money.

Fixed in #115

from money.