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

Figure out how we can introduce rendering of inheritance #108

Closed
jonaslagoni opened this issue Feb 24, 2021 · 9 comments
Closed

Figure out how we can introduce rendering of inheritance #108

jonaslagoni opened this issue Feb 24, 2021 · 9 comments
Labels
enhancement New feature or request keep Dont let stale get to it. stale

Comments

@jonaslagoni
Copy link
Member

Reason/Context

One of the strong features of the CommonModel is the possibility to use extend which contains a list of $ids of the CommonModels that the current model should extend. Therefore we need to figure out how we render this in the different languages

@jonaslagoni jonaslagoni added the enhancement New feature or request label Feb 24, 2021
@github-actions github-actions bot added the stale label Apr 26, 2021
@jonaslagoni jonaslagoni removed the stale label Apr 28, 2021
@github-actions github-actions bot added the stale label Jun 28, 2021
@jonaslagoni jonaslagoni added this to the Version 1.0.0 milestone Jul 27, 2021
@jonaslagoni jonaslagoni removed the stale label Jul 27, 2021
@github-actions github-actions bot added the stale label Sep 26, 2021
@jonaslagoni
Copy link
Member Author

Still relevant.

@jonaslagoni
Copy link
Member Author

Still relevant.

@jonaslagoni jonaslagoni removed the stale label Feb 7, 2022
@smoya
Copy link
Member

smoya commented Feb 7, 2022

Currently, supported inputs don't really support inheritance (we could discuss if JSON Schema somehow does, but it doesn't really do).

However, if we ever support any other language as input (like PHP for example), the generators will need explicit info about both a class extending and/or implementing an interface. See the following examples:

Class Iinheritance:

class Human extends Animal

Implementing an interface:

class Human implements LivingBeing 

But also

class Human extends Animal implements LivingBeing

And also the fact a class can implement several interfaces.

So, does this issue still applies? Do we want to support at model level? is it worth to support right now?

@jonaslagoni
Copy link
Member Author

I don't think we should focus on it now, as it seems like a feature and not a complete rewrite that is required to fully make this work. But good points to remember for the future 👍

@smoya
Copy link
Member

smoya commented Feb 9, 2022

I don't think we should focus on it now, as it seems like a feature and not a complete rewrite that is required to fully make this work. But good points to remember for the future 👍

Maybe you want to pop out this from Version 1 milestone?

@jonaslagoni jonaslagoni removed this from the Version 1.0.0 milestone Feb 9, 2022
@magicmatatjahu
Copy link
Member

@jonaslagoni @smoya I think that my idea should "fix" that problem #628

@github-actions github-actions bot added stale and removed stale labels Jul 31, 2022
@asyncapi asyncapi deleted a comment from github-actions bot Aug 10, 2022
@asyncapi asyncapi deleted a comment from github-actions bot Aug 10, 2022
@asyncapi asyncapi deleted a comment from github-actions bot Aug 10, 2022
@asyncapi asyncapi deleted a comment from github-actions bot Aug 10, 2022
@asyncapi asyncapi deleted a comment from github-actions bot Aug 10, 2022
@jonaslagoni jonaslagoni added the keep Dont let stale get to it. label Aug 10, 2022
@jonaslagoni
Copy link
Member Author

jonaslagoni commented Aug 10, 2022

I have been thinking a bit about this feature, and there is one huge problem with it, that I dont know if we want to introduce - and that is complexity.

When you write presets and use the class to anything, you MUST remember that it CAN extend from something else, and handle that accordingly. Which is a HUGE effort which needs to be put into Modelina, without much benefit IMO.

I can however see a reason we would want to introduce a preset hook (for now) that enables you to more easily extend from your own custom class, in case you have some use-case where that would suit you. But it does not force preset and generator creators to handle any of the complexity because it's not something thats added to the ConstrainedModel.

The complexity I am referring to is for example in serialize presets that given the following generated models in C#:

public class Person
{
    public string FName { get; set; }
    public string LName { get; set; }

    public Person(string fname, string lname)
    {
        FName = fname;
        LName = lname;
    }
}

public class Student : Person
{
    public Student(Person person, string code)
        : base(person.FName, person.LName)
    {
        this.code = code;
    }
    public Student(Person person)
        : base(person.FName, person.LName)
    {

    }
    public string code { get; set; }
}

Suddenly need to look into the superclass Person when serializing Student, it is very easy to forget this and takes quite a few lines of code to get right.

I dont know, I think we will have to see as the library grows whether this is really needed to be supported in full.

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity 😴

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 ❤️

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 15, 2024
@AnimeshKumar923
Copy link

still valid? @jonaslagoni

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request keep Dont let stale get to it. stale
Projects
None yet
Development

No branches or pull requests

4 participants