typelevel / squants

The Scala API for Quantities, Units of Measure and Dimensional Analysis
https://www.squants.com
Apache License 2.0
923 stars 122 forks source link

Money.apply should not use defaultCurrencyMap #296

Open veej opened 6 years ago

veej commented 6 years ago

Since now it's possible to define our additional currencies, I expect to be able to create a new Money passing a currency code from the additional ones I added, like in the example above:

object NMY extends Currency("NMY", "New Money", "$", 2)
implicit val myContext = moneyContext.withAdditionalCurrencies(Set(NMY))

val money = Money(10, "NMY")

However, the snippet of code above is throwing an exception, since Money.apply(amount: BigDecimal, currency: String) is using a static defaultCurrencyMap for the String-to-Currency conversion, instead of the list of currencies from the implicit context.

hunterpayne commented 5 years ago

I tried to make this work in my sandbox and ended up breaking the JS platform build in a way I don't understand. So this seems to be a difficult to fix issue and might require some larger refactoring of the market package. Also, the easy workaround is to directly use the currency object.

Perhaps an implicit conversion from String -> Currency and removing the apply methods that take currency as a String is a better fix in the short run. Not sure how that would work as implicit conversions should always work and this one would fail part of the time.