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

implement #2636: provide overloadings of ObjectReader.readValue() taking Class valueType #2637

Merged
merged 2 commits into from
Mar 19, 2020

Conversation

robinroos
Copy link
Contributor

@robinroos robinroos commented Mar 2, 2020

(to fix #2636)

This PR provides additional overloadings of ObjectReader.readValue() which take the "Class valueType" argument.

This serves to make a typecast unnecessary when the result of readValue is being passed to an overloaded method.

I also introduced a test, without assertions (!), which shows that the code compiles.

@cowtowncoder
Copy link
Member

Hmmh. I think this is against the design goals of ObjectReader, adding yet another overloading, and adding to combinatorial overload. In fact, it was designed to overcome it.
I'll have to read the issue itself to see, but right now I am not sure this is something I'd want to add.

@robinroos
Copy link
Contributor Author

robinroos commented Mar 2, 2020

@cowtowncoder thanks for your comment. Note that all of the valueOf() methods return <T> T, but that only the ones taking a JsonParser provide for binding of <T>. The rest of them may as well return Object.

@robinroos
Copy link
Contributor Author

@cowtowncoder, from my (user not developer) perspective, ObjectReader serves to provide a read-side view of ObjectMapper, specifically one which is based on immutable config (c.f. mutant factory as noted in its JavaDoc). Minimising the read-side API would not seem to be the primary objective of this class.

@robinroos
Copy link
Contributor Author

Noting that this just missed 2.10.3 (published yesterday), I'd appreciate it if this PR could form part of 2.10.4.

@robinroos
Copy link
Contributor Author

Acknowledging here that an API addition such as this would target a minor release (2.11.0) and not a point release.

@cowtowncoder cowtowncoder changed the title Issue #2636 - provide overloadings of ObjectReader.readValue() taking… implement #2626: provide overloadings of ObjectReader.readValue() taking Class valueType Mar 7, 2020
@cowtowncoder
Copy link
Member

cowtowncoder commented Mar 7, 2020

Ok, I will think about this. Number of overloads that would be necessary for full coverage of input types is rather big.

@robinroos
Copy link
Contributor Author

I only use the overloading of String argument-type, and added the others for completeness.

@cowtowncoder
Copy link
Member

As per my note on issue, I'll accept this patch: thank you for providing it!

Just one last process thing: unless I have asked for and received (apologies if so; I tried to check) CLA, I would need that now from:

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

(or Corporate CLA, CCLA, if you prefer, in same repo)

and usually it's easiest to print, fill, sign & scan, email to info at fasterxml dot com.
Only one CLA needed for all Jackson project contributions.

Looking forward to merging this!

@cowtowncoder cowtowncoder merged commit 7e7fe80 into FasterXML:master Mar 19, 2020
@cowtowncoder cowtowncoder modified the milestones: 1.9.13, 2.11.0 Mar 19, 2020
@cowtowncoder cowtowncoder changed the title implement #2626: provide overloadings of ObjectReader.readValue() taking Class valueType implement #2636: provide overloadings of ObjectReader.readValue() taking Class valueType Mar 19, 2020
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

Successfully merging this pull request may close these issues.

ObjectReader readValue lacks Class<T> argument
2 participants