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

Class mappings for ValueType enum should be mutable #130

Open
aseovic opened this issue May 30, 2018 · 4 comments
Open

Class mappings for ValueType enum should be mutable #130

aseovic opened this issue May 30, 2018 · 4 comments

Comments

@aseovic
Copy link
Contributor

aseovic commented May 30, 2018

While the default class mappings for ValueType enum make sense, there are cases when users may want to override them.

For example, we have our own class that all untyped JSON objects should map to, and a converter for it, but in order for the converter to kick in we need to be able to change mapping for the ValueType.OBJECT from Map.class to our class.

In a more general case, it may be useful to use javax.json.JsonObject as a default for the ValueType.OBJECT. I'd even argue that should be the default, but considering that changing the default would break backwards compatibility in a major way, allowing users to change it is probably the best we can do.

Finally, while ValueType.OBJECT is probably the most common value users will want to change the mapping for, I can see how changing the mapping for ValueType.DOUBLE to BigDecimal can come in handy in some cases.

@EugenCepoi
Copy link
Contributor

If someone wants to ser/de to other types when the static type is unknown the solution is to implement something like UntypedConverterFactory.

Isn't this enough?

@aseovic
Copy link
Contributor Author

aseovic commented May 31, 2018

Sure, that's an option, but it seems like an overkill when a simple remapping would do.

Implementing and registering custom converter is significantly more involved, so having an out-of-the box way to define the mappings from ValueType -> class that should be used for deserialization would be nice.

@EugenCepoi
Copy link
Contributor

The ValueType is supposed to reflect what has been read and not related to databinding. So changing it to accommodate databinding use cases feels wrong to me. Also note that for the double to bigdecimal example, in order to preserve this accuracy you will want to represent it as a json string. Genson will represent decimal numbers as doubles in JsonReader.

We can make it easier to do a remapping via the untyped converter factory (or another one) so it's not overkill anymore.

@aseovic
Copy link
Contributor Author

aseovic commented Jun 1, 2018

Fair enough, I agree that making ValueType mutable is a bit hacky, but it was the quickest way to achieve the immediate goal ;-)

Let me re-work it and implement a cleaner way to modify the mappings and I'll resubmit the PR.

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

No branches or pull requests

2 participants