Make BaseCurrencyProvider always return BigNumber

[rdarcy1]

When using BaseCurrencyProvider with another provider that doesn't store exchange rates as BigNumber, the return type is inconsistent between the original exchange rate and its reciprocal.

$configurableProvider = new ConfigurableProvider();
$configurableProvider->setExchangeRate('USD', 'EUR', 1.2);
$baseProvider = new BaseCurrencyProvider($configurableProvider, 'USD');

// Returns float
$rate = $baseProvider->getExchangeRate('USD', 'EUR');

// Returns BigNumber
$rate = $baseProvider->getExchangeRate('EUR', 'USD');

I can see the value in being able to store and return rates as float/int/string in other providers if you choose, but in this case you'll always get a type mismatch on reciprocals unless you're storing as BigNumber. To make things more consistent, this PR converts all rates to BigNumber when returning from BaseCurrencyProvider.

Note this is a breaking change.

[BenMorel]

Hi, I'm not sure to see a value in this change.

Whether BaseCurrencyProvider returns a float or a BigNumber, the ExchangeRateProvider contract is respected. Even if you change the implementation to always return a BigNumber, the caller will still have to assume that, given the contract, a number or a string can be returned.

I'd be more willing to discuss whether getExchangeRate() should return BigNumber, although this library has been designed with maximum convenience in mind, so I'm not sure whether this is a good idea.

By the way, I'd be interested to know your use case: I designed the ExchangeRateProvider interface, thinking that it would only ever be used by the CurrencyConverter. Do you actually use its output for something else?

[rdarcy1]

That's a fair point, and I suppose I'm not strictly obeying the contract in my code when I'm assuming a BigNumber is returned rather than checking the type each time in my code.

I'm using the ExchangeRateProvider in an ecommerce site, where the user can select currency. Currencies are loaded in from a database table and are all relative to a single base currency (that of the country where manufacturing occurs). For the most part, the exchange rates are passed into the currency converter like you say, but they're also displayed to users and stored in the database against orders which necessitates knowing the type at some points.

I was confused by the inconsistent types coming back before I figured out I had to store exchange rates as BigNumber, and the PR stems from me thinking there's really no situation where you'd want to get back a float for one direction and BigNumber for the other, even if it does still obey the contract.

No worries if you close, appreciate it's a breaking change 'fix' for something that's not necessarily broken! Many thanks for the package.

[BenMorel]

Thanks for the feedback. It's interesting to see how this library is used in the wild!

When designing the library, I assumed that you would only ever use the exchange rate providers indirectly to perform conversions with the CurrencyConverter. I'm actually pleased to know that BaseCurrencyProvider can be useful on its own as well, and this forces me to re-think my approach.

Now that I think about it, covariance allows us to change the return type of BaseCurrencyProvider to BigNumber. This works right now with no return type set: https://3v4l.org/tNuOT

And will also work with PHP 8 union types: https://3v4l.org/B4PZe

Given that BaseCurrencyProvider is final, we can make this change right now with no BC break.

So please go ahead and add the BigNumber return type!

[rdarcy1]

Great, added return type.

[BenMorel]

LGTM, thanks @rdarcy1!

[BenMorel]

Released as 0.5.1!