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

Support for multiple constructors #102

Open
kenston opened this issue Aug 6, 2016 · 8 comments
Open

Support for multiple constructors #102

kenston opened this issue Aug 6, 2016 · 8 comments

Comments

@kenston
Copy link

kenston commented Aug 6, 2016

Is it possible to add support for deserializing to a class with multiple constructors where Genson will loop through the possible constructors and choose which one is appropriate based on the matching between JSON properties and constructor arguments?

@EugenCepoi
Copy link
Contributor

At some point Genson was doing it but it led to ambiguous situations where you don't know what constructor to pick. Then I decided on purpose to use only one constructor and don't switch at runtime.

Do you have some strong use case/argument for this feature? Perhaps we can find some alternative.

On Aug 6, 2016, at 2:10 AM, Ken Choi [email protected] wrote:

Is it possible to add support for deserializing to a class with multiple constructors where Genson will loop through the possible constructors and choose which one is appropriate based on the arguments?


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@kenston
Copy link
Author

kenston commented Aug 7, 2016

I'm using Genson to configure the creation of Java objects similar to how beans are created using Spring XML configuration. There are cases where classes have multiple constructors but with different argument names and types. Maybe we can have an option at GensonBuilder to allow us to decide whether to offer that functionality or not.

@EugenCepoi
Copy link
Contributor

Are the properties also settable using a setXx method? If so you can just pick one constructor and mark it with JsonCreator annotation. Genson then will use only this constructor and the setters for the other fields. You can also allow deser to private properties, which should work also in the case of final properties. Let me know if any of those would work for you.

@kenston
Copy link
Author

kenston commented Aug 8, 2016

The properties does not have or are not settable using a setXx method. My class have one or more usable constructors; thus, I can't just pick one over the other by placing the JsonCreator annotation. I also want to keep my classes clean from annotations just like how Spring supports it that way.

@ErikLundJensen
Copy link

ErikLundJensen commented Sep 23, 2016

We have the same issue. When Genson selects a constructor then it should at least select one that does not fail. Today it selects one constructor and then fails with NullPointerException if not all arguments are available.
Genson should continue to the next constructor to see if that one matches.

@EugenCepoi
Copy link
Contributor

Initially we had support for multiple constructors, here is why I removed it:

  • When two constructors match the defined properties, then we again don't know which one to pick.
  • If we imagine there are setters for the other properties then maybe it wouldn't matter what constructor we choose. But in this case why not have a single constructor (or many but this one annotated with JsonCreator) that takes all the required properties and uses setters for the ones that are optional?
  • It added complexity and performance overhead

What do you think, especially about point 2?

@ErikLundJensen I prefer we first discuss all this a bit before considering an implementation. I am not necessarily against it, but if we do so then it must add very low overhead to the performances.

@ErikLundJensen
Copy link

Sorry for not returning before. I understand that you want to keep the code smooth and fast.
And you are right, you should not solve problems caused by others not using annotations correctly.

I suggest that I delete my pull request and thereby make sure it is not used by others. Is that Okay?

@EugenCepoi
Copy link
Contributor

I am going to close your PR for now, but again, if this is a feature you strongly need let's discuss it. I am open to consider it :)

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

3 participants