Adding Single Int Constructor Support (Issue #25) #121
Conversation
Shounaks
force-pushed
the
2.17-singleint-constructor-support
branch
from
205fef7 to
b4e6fa3
Compare
| Set<String> ignorableNames, Map<String, String> aliasMapping) | ||
| { | ||
| super(type); | ||
| _propsByName = props; | ||
| _defaultCtor = defaultCtor; | ||
| _stringCtor = stringCtor; | ||
| _longCtor = longCtor; | ||
| _intCtor = intCtor; |
There was a problem hiding this comment.
I believe propogation of HashMap<FIRST_ARG,CONSTRUCTOR> should be a better alternative, to support future items. instead of adding individually in all the following files.
There was a problem hiding this comment.
I agree with the general idea (could use EnumMap too), but how about going even more OO and have Constructors (or whatever its named) container that encapsulates these details?
There was a problem hiding this comment.
I tried using enum map, but that would require us explicitly manage a list of supported items, with the code getters and setters; a problem not present in HashMap.
| @@ -6,12 +6,12 @@ | |||
| // for [jackson-jr#25], allowing single-int constructors | |||
| public class ReadWithCtors25Test extends TestBase | |||
There was a problem hiding this comment.
need to move this to passing tests, once the implementation is approved
cowtowncoder
added
the
has-failing-test
There was a problem hiding this comment.
Ok, yes, this makes sense -- but I think the idea of creating separate container class for different constructors makes sense: can then pass that object instead of 4 constructors (plus obv easier extension in future, as necessary).
With that I'll be happy to merge this!
| @@ -0,0 +1,29 @@ | |||
| package com.fasterxml.jackson.jr.ob.impl; | |||
There was a problem hiding this comment.
Um, sorry, but this looks unnecessarily complicated: instead of Map, suppliers etc, let's just stick to plain old typed fields. Map adds overhead while not improving readability.
Same goes for exception, construction: straight-forward non-generalized construction, exception.
|
Thank you for contributing this PR! I created slightly modified one based on my preferences wrt Constructor collector, merged: since it is based on your code I put you as the author in release notes. I'll close this PR as done. |
(Issue #25)
this already had a failing test (we should add this tag)