-
Notifications
You must be signed in to change notification settings - Fork 767
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
Feature Request: A more flexible PolymorphicJsonAdapter #1273
Comments
I think a better API would be to read the entire object as a json value and then pass it to a lambda for each subtype. PolymorphicJsonAdapter.builder(Account::class.java)
.add(User::class.java, { it["label"] == "has_verified_email" })
.add(SuspendedUser::class.java, { it["is_suspended"] == true })
.add(DeletedUser::class.java, { it["is_deleted"] == true })
.build() That being said, is this a good use of polymorphism? What if you want to differentiate suspended users who have verified emails? The polymorphic support is for when the fields of a type are unrelated to each other and discerned by a discriminator. It is not for mapping state of an object which fundamental represents the same thing into some kind of hierarchy. You're probably going to be on your own for this one... |
Hi Jake, While creating the class I wanted to stay close to the original, but if I had to read the whole object we could do it like this:
Much better. I thought of this example on the spot since I cannot share company code, maybe I could have been clearer. The issue at the center are those situations where you have to discern between subclasses but:
If I had a say in the matter I would add a field with the type and use PJA as it is, but oftentimes you just have to deal with a bad API and you need a more complex logic to identify the subclass. I think covering more situations could be beneficial, but then again if you say I'm on my own... |
I don't think there is a need to map these states as different concrete models. |
There is no concrete need, but there is a consideration to do about nullability. In my mind it's easier to have submodels to bind around. Others might have a different opinion. |
Nothing more to do here, if you need this kind of customization it's better to write your own. |
Hi,
I wanted to start a conversation about how to make Polymorphic Deserialization better in the future.
Sometimes you find yourself dealing with a very bad API that cannot be changed, cause you don't have access to the backend, and in those cases, I found the PolymorphicJsonAdapter approach to be challenging to use.
Let's look at this example:
representing:
Dealing with this would be very simple:
But now let's think for a moment about this situation:
representing:
This situation is much more problematic to handle, for two reasons:
Following #1094 there is a way to control unknown labels, with fallbackJsonAdapter, but if you have a lot of these cases your code might end up messy. Also if you generate your adapters with code-gen, the classes take in a Moshi instance, so you would need one to build another? Very cumbersome.
Since I was dealing with such cases very often, I developed an alternative method:
That can be used like this:
This is not the best code ever written, not the prettiest, but it's working in almost any situation.
Can we build a more futurable adapter that maybe uses a Matcher<> class comparing labels and values, so we can build logic to identify what subtype we are dealing with, in a simpler way?
What do you guys think?
Do you have a better solution?
Thanks in advance
The text was updated successfully, but these errors were encountered: