Closed frederikbosch closed 3 years ago
@sagikazarmark @Ocramius This PR and probably a PR related to calculators will be the last PRs before tagging version 4.0 beta 1. Since @Ocramius contributed so much already, and therefore has specific knowledge of the upcoming release, I would kindly like to ask him if he is prepared to review this PR.
I read through it, but naming/API does not really makes much sense to me 🤔
I agree. The names of the methods in the Converter
class are too close. convert
must stay for BC reasons, and I think that name fits the purpose. Maybe I should be more explicit for convertAgainst
and change it into convertAgainstCurrencyPair
.
But then what to do with conversion
? I tried to lookup if there is a name inside the exchange domain that fits this purpose but I was not able to find it. It's like you stand at the counter of the exchange office and you have just received money in the new currency with a receipt that shows you the currency pair. What do you say of convertWithReceipt
and return [Money, CurrencyPair]
?
The PR was updated with improved method names.
Fixes #498, supersedes #502.
This PR provides:
Converter#conversion
to get a combination ofMoney
and theCurrencyPair
that was used for conversion.Converter#convertAgainst
, to convert against a knownCurrencyPair
, and does make any call to an exchangeWhy? When an amount is the result of a conversion, this amount usually saved together with the conversion rate. The method
Converter#conversion
helps to fulfill this need directly. WhileConverter#convertAgainst
helps to convert an amount based on a savedCurrencyPair
.