Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Money.apply should not use defaultCurrencyMap #296

Open
veej opened this issue Feb 5, 2018 · 1 comment
Open

Money.apply should not use defaultCurrencyMap #296

veej opened this issue Feb 5, 2018 · 1 comment

Comments

@veej
Copy link

veej commented Feb 5, 2018

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
Copy link
Contributor

hunterpayne commented Jan 19, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants