Something is wrong with the compare method in Calculator

[ghost]

when doing divide, it'll call the calculator to verify the divisor (cast to string first) isn't zero.

from what I see, the compare method in BcMathCalculator doesn't have scale as third parameter.

bccomp($a, $b)

which makes bccomp("0.5", 0) === 0: "Returns 0 if the two operands are equal"

if use GmpCalculator instead,

ErrorException: gmp_cmp(): Unable to convert variable to GMP - string is not an integer

[frederikbosch]

Why would you want that in a Money context? It is impossible to construct a Money object with a decimal.

[frederikbosch]

The same story is true for almost any gmp_ method. You cannot substract or add a decimal number with the current GMP calculator. But since adding and subtracting decimals is not required by this package either (for the same reason I already gave that it is impossible to construct a Money object with a decimal), there will not be support for it until we start working on PreciseMoney. See #7.

[frederikbosch]

Also relevant in this thread is the GMP Floating Point Support RFC.

[sagikazarmark]

We could probably throw a logic exception when doing operations with
decimals.

Frederik Bosch [email protected] ezt írta (2016. október 1.,
szombat):

Also relevant in this thread is the GMP Floating Point Support RFC
https://wiki.php.net/rfc/gmp-floating-point.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#278 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABK2kFZ6L7zaXWqicVwLXy3S2Q-x38Dlks5qvkAEgaJpZM4KJusn
.

[frederikbosch]

@sagikazarmark I think we should just leave it. If there is no Money problem, there is no problem. We already have insufficient hands to get 3.0 out in reasonable time. If we starting fixing these issues...

[sagikazarmark]

Well, I agree. Honestly, I am not sure I completely understood the problem. However, if there is a leak in the API which easily leads to errors like this AND fixing them requires an API change we should do something.

Again: not sure if this is the case.