Introduces registry for localisers & exposes them via Configurator #243
Conversation
… via Configurator, and made the default factories lazy loaded
…l classes and properties
|
Looking good 👍 |
| { | ||
| _defaultLocaliser = MakeLazy(defaultLocaliser); | ||
| _localisers = new Dictionary<string, Lazy<T>>(); | ||
| foreach (var localiser in localisers) |
|
LGTM |
|
Whoa!! You guys are pretty quick. I had an idea right after I created the PR and didn't think you'd review this fast! I made a few more changes that simplifies the registration and cleans up registry's constructors a bit. I'd appreciate another review. Sorry :p |
| @@ -679,6 +680,11 @@ ByteSize.Parse("1.55 tB"); | |||
| ByteSize.Parse("1.55 tb"); | |||
| ``` | |||
|
|
|||
| ##<a id="configuration">Configuration</a> | |||
|
|
|||
| Custom factories for `Formatter`s, `NumberToWordsConverter`s, and `Ordinalizer`s may be added or updated via `FormatterFactoryManager`, `NumberToWordsConverterFactoryManager`, and `OrdinalizerFactoryManager` using the `SetFactory` method. | |||
There was a problem hiding this comment.
Oh, I should apply the changes to readme too if you guys are happy with this API.
|
Writing this on my phone so sorry if the formatting comes out poorly. That definitely may fall under YAGNI/don't care, though, and if it becomes an issue, as long as the public signature remains I actually woke up with the idea of offering 2 overloads for my At this point, I'd say FISI and revisit if it becomes an issue. |
|
@hazzik - thanks for the review. Why do you not like @justin-edwards - thanks for the feedback. I think summing up your and @hazzik's feedbacks perhaps we could just add an overload with |
|
Thanks for the feedback guys. I added a few more commits to address the issues. What do you think? |
|
|
IMO.
hazzik
changed the title
|
Merged now. Thanks for your input guys & thanks for the great effort @justin-edwards. |
Based on #227, that fixes #150 and #218, I made some changes to the signature of the factories based on the discussions we had over at #227, added missing xml documentations to fix the build & fixed the broken tests due to Serbian formatter gone missing on the rebase.
I'd appreciate your review and feedback /cc @justin-edwards @hazzik @mexx