spec
spec copied to clipboard
effect of inheritance / polymorphism on code generators
Hello AsyncAPI community,
with issue #286 an example for declaring a discriminator
has been added to the spec. Following this example, I've created an asyncapi file and tried to generate code using the java-spring-template. My expectation had been to find generated classes Pet
, Dog
and Cat
and I further expected the two latter classes to inherit from Pet
, so the petType
would be available as a property of them.
However, the code generator template doesn't work that way. It always uses aggregation.
In this comment I have explained why I find it hard to deduct the correct "result expectation" from the specification:
the code is not going to be what you expect,
petType
property will not be available in Cat and Dog classes
Correct, that's what I would have expected; instead, it generates AllOfPetCat
.
[...]
I think this default behaviour is correct. As I understand you expect that in case of discriminator usage, it should be supported?
In fact, I rather think that the specification is unclear about that:
While composition offers model extensibility, it does not imply a hierarchy between the models.
I interpret that as a way of saying "code generators SHOULD NOT express it with inheritance".
There are are two ways to define the value of a discriminator for an inheriting instance.
That implies "When the discriminator is present, code generators MUST establish inheritance". The spec then differentiates two examples.
- The first one with the naming choice of
ExtendedErrorModel
looks like a perfect fit for composition / aggregation. - The second one with the
Pet
looks (at least to me) like a perfect fit for inheritance. My assumption about inheritance is also re-enforced by the section that describes thediscriminator
:
The discriminator is the schema property name that is used to differentiate between other schema that inherit this schema. [...] When used, the value MUST be the name of this schema or any schema that inherits it.
But the spec doesn't give any hint how inheritance shall be represented, thus leaving room for interpretation (or confusion :) ).
Following the guiding principle "Understandability is just as important as correctness" I would like to ask you for your clarification on what is the best fitting interpretation for code generator when translating the pet example to actual code.
In addition, "Pet" is probably not a perfect example for class inheritance: It depends on the application context whether "Pet" is considered to be in the "nature" of Dog and Cat, or just seen as a "role" that they take (as compared to the schoolbook example of "Truck" and "Car" clearly inheriting from "Vehicle").
Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
This issue has been automatically marked as stale because it has not had recent activity :sleeping: It will be closed in 60 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation. Thank you for your contributions :heart:
I had been off for a while, but I'd still be very much interested in your expert clarifications on how to interpret the Pet example in generated code.
Thanks for adding it to the milestone, @fmvilas. @gibson042, you are the one who added the example in the spec which this issue refers to. Do you like my proposal from the end of the issue description (Truck / Car / Vehicle instead of Dog / Cat / Pet) to achieve more clarity?
Do you like my proposal from the end of the issue description (Truck / Car / Vehicle instead of Dog / Cat / Pet) to achieve more clarity?
Yes. :+1:
Is there any of you who wants to champion this? 🙂 Or can we consider this issue as needs champion? 🤔
This issue has been automatically marked as stale because it has not had recent activity :sleeping:
It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.
There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.
Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.
Thank you for your patience :heart:
This issue has been automatically marked as stale because it has not had recent activity :sleeping:
It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.
There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.
Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.
Thank you for your patience :heart: