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 polymorphism #103

Open
macisamuele opened this issue Feb 7, 2020 · 3 comments · May be fixed by #104 or #144
Open

Support polymorphism #103

macisamuele opened this issue Feb 7, 2020 · 3 comments · May be fixed by #104 or #144
Labels
enhancement New feature or request

Comments

@macisamuele
Copy link
Collaborator

macisamuele commented Feb 7, 2020

Swagger specifications allow models to be polymorphic via a discriminator field (doc).

This allows a model, let's say Animal to be a sort of parent in the hierarchy of other types like Dog, Cat, etc.

The polymorphic approach is generally interesting when an endpoint is returning a base object (ie. Animal) that can be specified more detailed by related type or when your endpoint returns a list of mixed objects.

Retrofit example

class Animal(open var type: String)
data class Dog(override var type: String, var name: String): Animal(type=type)
data class Cat(override var type: String, var size: Int): Animal(type=type)

@JvmSuppressWildcards
interface SwaggerApi {
    @GET("/animals")
    fun getAnimals(): Single<List<Animal>>
}

The endpoint GET /animals might be returning a JSON similar to

[
    {"type": "dog", "name": "name"},
    {"type": "cat", "size": 1},
    {"type": "whale", "weight": 42}
]

and ideally we should be able to decode it as

listOf(
    Dog(type="dog", name="name"),
    Cat(type"cat", size=1),
    Animal(type="whale")
)

Without real support for polymorphism (as for version 1.3.0) the response would be decoded as

listOf(
    Animal(type="dog"),
    Animal(type="cat"),
    Animal(type="whale")
)

which leads to some information loss (the dog name was sent to the client and the client should have known about it)

Disclaimer: The above presented example is mostly to help understand the feature that polymorphism brings to codegen.
An other possibility might be to define inheritance via sealed classes

sealed class Animal(open var type: String) {
    data class DefaultAnimal(override var type: String): Animal(type=type)  // How to pick a name that has not been used yet?
    data class Dog(override var type: String, var name: String): Animal(type=type)
    data class Cat(override var type: String, var size: Int): Animal(type=type)
}

Does someone have other ideas?

@cortinico
Copy link
Collaborator

Seems like the first example doesn't compile as the Animal class is not open:

open class Animal(open var type: String)
data class Dog(override var type: String, var name: String): Animal(type=type)
data class Cat(override var type: String, var size: Int): Animal(type=type)

Anyway this sounds like the easier approach to follow, at least for now. Having sealed classes will help protect the Animal class from other subclasses that could be created by the user. On the other end it probably over complicates the generator.

@raamcosta
Copy link

Hi guys, what about using interfaces as the supertype?

interface Animal(var type: String)
data class Dog(override var type: String, var name: String): Animal
data class Cat(override var type: String, var size: Int): Animal

Or

sealed interface Animal(var type: String)
data class Dog(override var type: String, var name: String): Animal
data class Cat(override var type: String, var size: Int): Animal

(Dog and Cat can be nested inside Animal or not in this case. I would almost always prefer it to not be nested, but this is of course totally discussable)

I feel like interfaces work better with data classes and are exactly what we need for this. I have used this approach in big inheritance hierarchies with data classes where every inflection point is modeled with an interface and it has worked great for me.

Btw a bit of a side question, please point me to somewhere if this was already discussed or if it is configurable. Why are we using var instead of val?
If I were writing this code by hand, I would never use var. Or is it a limitation of the json parsing approach?

@cortinico
Copy link
Collaborator

I feel like interfaces work better with data classes and are exactly what we need for this.

Agree. When we initally discussed the Poly support, sealed interfaces were not a thing in Kotlin yet. So definitely worth to consider and I think they would be the best fit here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
3 participants