spec icon indicating copy to clipboard operation
spec copied to clipboard

effect of inheritance / polymorphism on code generators

Open patrickp-at-work opened this issue 3 years ago • 7 comments

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 the discriminator:

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").

patrickp-at-work avatar May 27 '21 09:05 patrickp-at-work

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.

github-actions[bot] avatar May 27 '21 09:05 github-actions[bot]

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:

github-actions[bot] avatar Jul 27 '21 00:07 github-actions[bot]

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.

patrickp-at-work avatar Jul 27 '21 06:07 patrickp-at-work

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?

patrickp-at-work avatar Sep 22 '21 18:09 patrickp-at-work

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:

gibson042 avatar Sep 22 '21 22:09 gibson042

Is there any of you who wants to champion this? 🙂 Or can we consider this issue as needs champion? 🤔

jonaslagoni avatar Jan 19 '22 18:01 jonaslagoni

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:

github-actions[bot] avatar May 25 '22 00:05 github-actions[bot]

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:

github-actions[bot] avatar Jun 27 '23 00:06 github-actions[bot]