-
-
Notifications
You must be signed in to change notification settings - Fork 261
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
Reduce boilerplate needed for IValueParser implementations #169
Reduce boilerplate needed for IValueParser implementations #169
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making this PR. I'd be happy to get this merged. I have a few comments I'd like to discuss first, but nothing too major. We should be able to get this in soon.
Also, it looks like some of the unit tests are failing. Can you take a look?
Strange. Passed on mine before I opened the PR but now I see it's failing. Bizarre. Will have a look for and submit the fix. |
I've addressed this in aec7e57 but looks like build is now failing due to the unrelated commit 4128af0 in On my box, the tests are now all passing:
|
This PR adds
ValueParser.Create
that is designed to reduce the amount of ceremony needed forIValueParser
andIValueParser<>
implementations.ValueParser.Create
and its overloads return a standard implementation ofIValueParser
andIValueParser<>
that delegate to a single parsing function.As a testimony of the code reduction and increased re-use, all of the following:
BooleanValueParser
ByteValueParser
DoubleValueParser
FloatValueParser
Int16ValueParser
Int32ValueParser
Int64ValueParser
StringValueParser
UInt16ValueParser
UInt32ValueParser
UInt64ValueParser
UriValueParser
have been replaced with singletons in fields of
StockValueParsers
, as in:Within
StockValueParsers
, there's even greater consolidation of all duplication amongst categories (for integers, non-negative integers & floating-point) of value parsers.All unit tests are passing.
Forgive me for submitting a PR before opening an issue but I thought it'd be okay per the following point in the contribution guidelines:
This PR does not change behavior. It doesn't change any existing API. It does however add one new method (that is
ValueParser.Create
plus overloads) to help users of this library to implement new value parsers in their CLI applications without the ceremony of implementing theIValueParser
/IValueParser<T>
interface.