brick / money

A money and currency library for PHP
MIT License
1.63k stars 102 forks source link

There are equals method? #17

Closed solodkiy closed 4 years ago

solodkiy commented 6 years ago

I need a simple bool method what answer me are the two money object completely equal (amount and currency) or not. isEqualTo looks like this method, but it throws MoneyMismatchException exception if currencies are different. What I should use for this problem?

solodkiy commented 5 years ago

Option 1

Still think that exception in equals is side effect and you don't need to rely on it. If the same currency is important for you you should throw exception even before comparison right after input. Example:

function work() {
  $a = loadMoneyA();
  $b = loadMoneyB();
  // Need assertion there
  $id = saveToFile($a, $b);

  if ($a->isEqualTo($b)) { // Because there can be too late.
    sendMessage($id);
  } 
}
jiripudil commented 5 years ago

As already stated: Option 3 and isAmountAndCurrencyEqualTo()

jkuchar commented 5 years ago

Option 3 and is() as it is descriptive on its own

BenMorel commented 4 years ago

Happy new year! 🎉 I'd like to revisit this issue and fix it once and for all.

Re-reading the thread more than one year later, I'm now torn between:

What lead me to revisit the second point is @jkuchar's comment above, in particular:

This implies that isEqualTo (equality realation) is always defined for every two Money objects. This is because it does not depend from which class of equivalence value come from.

However if we are comparing values (greater then, lower then, ...) we need to have both values coming from the same class of equivalence, otherwise greatherThen and lowerThen is not defined.

My issue with this, however, is twofold:

Some fresh thoughts?

jkuchar commented 4 years ago

It still makes sence to me what I have written. Equality and comparison are two different things to me.

Imagine you are adding Money to hash map. When hash collision happens, collection asks if these two are equal, if not it adds second one there. What is the reason to throw an exception if they are two different currencies? 🤔

BenMorel commented 4 years ago

What do you with the isEqualTo() / isGreaterThanOrEqualTo() behaviour inconsistency, though?

BenMorel commented 4 years ago

As we're not getting anywhere here, I've finally released 0.4.4 with a new method: isAmountAndCurrencyEqualTo().

This may or may not change in the future; I understand that this solution does not make perfect sense from a mathematical standpoint, but it's practical, non-breaking, and the other proposed solution has its inconsistencies, too (at least IMO).

And after nearly 2 years without a concensus, I prefer having something non-perfect, than having nothing at all.

I'm closing this issue now, but I'm open to comments, in this thread, or preferably in a new one, if someone wants to bring new arguments to the table.

Thanks to all for your participation.