Issues with comparison - returns false although both are the same

[madsem]

Hey Ben,

I'm running into an issue where I'm unsure if we're looking at a bug. When comparing a Money to an int, it returns false as in it's not the same, although it is.

I have these dumps in my phpunit tests:

# $this->min_bid = Money obj of 0.06 USD
# $this->optimized_bid = int 6

dump($this->min_bid);
dump($this->optimized_bid);
dump($this->min_bid->isGreaterThanOrEqualTo($this->optimized_bid));

This is the output:

Brick\Money\Money {#3601
  -amount: Brick\Math\BigDecimal {#3612
    -value: "6"
    -scale: 2
  }
  -currency: Brick\Money\Currency {#3594
    -currencyCode: "USD"
    -numericCode: 840
    -name: "US Dollar"
    -defaultFractionDigits: 2
  }
  -context: Brick\Money\Context\DefaultContext {#3608}
}
6
false

I'm getting the same result when comparing with isEqual. This might be me misunderstanding how this is supposed to work, but to me the result is unexpected.

is 6 equal to 6? yes, or not? lol

[madsem]

I did some more testing:

#min_bid = Money 0.06 USD

#returns false
dump($this->min_bid->isGreaterThanOrEqualTo(5));

# returns true
dump($this->min_bid->isGreaterThanOrEqualTo(Money::ofMinor(5, 'USD')));

It might be wrong, but what is throwing me off is that the method describes float|int as valid arguments:

/**
     * Returns whether this money is greater than or equal to the given amount.
     *
     * @param AbstractMoney|BigNumber|int|float|string $that
     *
     * @return bool
     *
     * @throws MathException          If the argument is an invalid number.
     * @throws MoneyMismatchException If the argument is a money in a different currency.
     */
    final public function isGreaterThanOrEqualTo($that) : bool
    {
        return $this->getAmount()->isGreaterThanOrEqualTo($this->getAmountOf($that));
    }

So I think that either the phpdoc is wrong or the method(s) aren't working as they're supposed to :)

[BenMorel]

Hi, what you're comparing is 6 USD vs 0.06 USD. So obviously they're not equal!

The internal value you see for the BigDecimal is 6 with a scale of 2, which means 0.06.

Some examples of how this works:

Money::of(5, 'USD')->isEqualTo(5); // true
Money::ofMinor(5, 'USD')->isEqualTo('0.05'); // true

of() accepts a value in dollars, while ofMinor() accepts a value in cents.

isEqualTo()/isGreaterThanOrEqualTo()/..., if given a number, assume that the amount is in USD, not in cents.

If you want to compare against a number in cents, you can use Money::getMinorAmount(), which returns a BigDecimal in cents.

If this is unclear in the docs, maybe a PR to improve them could be useful?

[madsem]

That does make sense kinda, but still think this should be noted in the phpdoc maybe?

It's not clear at all that only in the case of a minor Money, any INT amount is considered a full amount :)

I'll gladly make a PR, would you prefer this in the README or in the phpdocs of the two comparison methods?

[BenMorel]

It's not clear at all that only in the case of a minor Money, any INT amount is considered a full amount :)

Not sure what you mean here: there is no "minor Money", just a Money that can be constructed from an amount in currency units (e.g. USD) with of(), or from an amount in minor currency units (e.g. cents) with ofMinor(). Both result in a Money instance, e.g. these two examples are equivalent:

Money::of(5, 'USD'); // USD 5.00
Money::ofMinor(500, 'USD'); // USD 5.00

When using the comparison methods, you always compare against the currency unit (e.g. USD).

Are you coming from moneyphp.org? You may be confused by the fact that they always deal with cents, if I'm not mistaken. This library is different.

[madsem]

No i'm actually using a Money object for the first time :). I've spent the last 1.5 years coding night and day to learn it.

So like I said from the beginning, it's very possible I don't understand it haha.

But what threw/throws me off still is, that the comparison methods accept float and int.

And a Money as you also said, is a Money. If it was created from float, or a minor int amount doesn't matter, it is saved as an int amount in any case.

So if I then make a comparison with an INT as well, i would expect it to work like I tried. Because the actual amount is the same.

Scientifically speaking, you are of course correct, the currency is important.

But imho, only if you compare a Money to another Money...

It i as user, compare an amount it is my responsibility to ensure that what i'm comparing makes sense. Which in my case I did, the 6 cents are USD they just come from a raw DB query and do not go through the ORM.

And because it's a lot of data I work with, I didn't want to new up thousands of Money objects. I simply wanted to make sure that the amounts are greater than, or equal to.

Does that make any sense? 😂

[BenMorel]

And a Money as you also said, is a Money. If it was created from float, or a minor int amount doesn't matter, it is saved as an int amount in any case.

Nope, it's always stored as a BigDecimal that represents an amount in the currency unit (USD here), that by default has the official scale for the currency (2 for USD), but may have many more digits if you provide a Context object:

Money::of('1.23', 'USD', new CustomContext(4)); // USD 1.2300
Money::ofMinor('1.23', 'USD', new CustomContext(4)); // USD 0.0123

The latter should be understood as 1.23 cents, expressed in USD with 4 decimals.

Money is always expressed in the currency unit you provide it with, and its amount is stored in this currency, not in cents (don't be confused by the fact that ofMinor() offers a convenient way to build a Money from an amount in cents: the result is still in USD) and therefore all comparisons with numbers, be them float or int, are made relative to the amount in the given currency:

Money::of(1.0, 'USD')->isLessThan(2); // true
Money::of(1, 'USD')->isLessThan(2.0); // true
Money::ofMinor(100, 'USD')->isLessThan(2.0); // true

You should understand all these examples as "is 1 USD less than 2 USD".

It i as user, compare an amount it is my responsibility to ensure that what i'm comparing makes sense. Which in my case I did, the 6 cents are USD they just come from a raw DB query and do not go through the ORM.

And because it's a lot of data I work with, I didn't want to new up thousands of Money objects. I simply wanted to make sure that the amounts are greater than, or equal to.

If you're storing monetary amounts in your database as cents (in most cases you should), then you have to reconvert them to a Money through ofMinor() before doing any kind of computation, including comparison:

$a = Money::ofMinor($aCentsFromDB, 'USD');
$b = Money::ofMinor($bCentsFromDB, 'USD');

$a->isGreaterThan($b);

... will work as expected.

If you really want to compare to cents directly, you could do this:

$money = Money::ofMinor($amountInCentsFromDB, 'USD');
$money->getMinorAmount()->isGreaterThan($otherAmountInCentsFromDB);

This would work, as getMinorAmount() will return a BigDecimal in cents, that can be compared to any number. But you lose a lot in expressiveness, and IMO raise the chances of making mistakes somewhere in your code, for dubious motives.

Does that make any sense? 😂

I'm not sure... :wink: