Allow currency parsing from implicit context #330
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This adds new method in the Currency companion to parse a Currency from a String given a MoneyContext. The method returns a Try[Currency], which is a Failure when the string code is not found in the set of currencies available in the MoneyContext. It also makes all methods in the Money companion that used to parse the currency symbol directly use instead the Currency companion and receive an implicit MoneyContext.
|
It looks like I missed this #305 before creating this PR. The comment there:
I agree with @estsauver when he says that the expected usage would be to have the context passed implicitily. Either way, I think the most important point would be to stop using the default context on those operations. Hope to hear some input on this. |
* master: Fixed Temperature's toString so it adds the space for the non-degree Kelvin unit, but keeps out the space for others. Also updated the TemperatureSpec to reflect this change. Changed the Kelvin symbol from "°K" to "K"
|
Hi there, |
garyKeorkunian
approved these changes
Feb 12, 2019
This was referenced Feb 12, 2019
|
@garyKeorkunian I've checked the current master (with this PR merged) and it seems like #323 is still a problem. |
|
Also getting that #323 when running tests |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a companion object to
Currencywhoseapplyreceives a currency code String, an implicitMoneyContextand returns aTry[Currency].It also changes the methods in
Moneycompanion that used to parse a currency from a String and relied solely on thedefaultMoneyContextto fetch the correspondingCurrencyvalue: these methods now receive an implicitMoneyContextand use the newCurrency.applywhich, I think, should solve #296.For my use case, these modifications are useful when working with domain models that specify some
Currency, that must be parsed from JSON, and when loadingCurrencyfrom typesafe config (using PureConfig).