modelina icon indicating copy to clipboard operation
modelina copied to clipboard

Properties that use const should be initialized with that const value and not be included in the constructor of a model

Open kennethaasan opened this issue 3 years ago • 1 comments

Description

Properties that use const should be initialized with that const value and not be included in the constructor of a model.

Example: When we set const to Cat in https://github.com/asyncapi/modelina/blob/next/test/generators/typescript/TypeScriptGenerator.spec.ts#L401, the generated class for Cat should automatically set the type property to PetType.CAT

https://github.com/asyncapi/modelina/blob/next/test/generators/typescript/snapshots/TypeScriptGenerator.spec.ts.snap#L7 should be:

class Cat {
  private _petType: PetType = PetType.CAT;
  private _reservedName: string;
  private _huntingSkill: AnonymousSchema_6;

  constructor(input: {
    petType: PetType, // this line should be deleted because you should not be able to set petType to something other than Cat
    reservedName: string,
    huntingSkill: AnonymousSchema_6,
  }) {
    this._petType = input.petType;
    this._reservedName = input.reservedName;
    this._huntingSkill = input.huntingSkill;
  }

  get petType(): PetType { return this._petType; }
  set petType(petType: PetType) { this._petType = petType; } // this is not needed because you should not be able to set petType to something other than Cat

  get reservedName(): string { return this._reservedName; }
  set reservedName(reservedName: string) { this._reservedName = reservedName; }

  get huntingSkill(): AnonymousSchema_6 { return this._huntingSkill; }
  set huntingSkill(huntingSkill: AnonymousSchema_6) { this._huntingSkill = huntingSkill; }
}

kennethaasan avatar Oct 12 '22 10:10 kennethaasan

I think it makes sense to add such support yes 🙂

I think we have to distinguish between two different concepts, default and constant.

default to me means that this is the value that the property default to, but you are allowed to change it. constant to me means that this is the value that the property takes, and you are not allowed to change it.

So for your enum case, I would say introducing constant makes sense (maybe even both 🤔).

But where to place such a value?

Placing it in the EnumModel, would mean (in the current setup) that if two properties referenced the same enum (maybe even across multiple classes) could not have different values i.e. PetType.CAT and PetType.DOG. We could of course in the end ensure they are duplicate instances of each other so two references don't have access to the same instance. But that seems... Like a step back in some way 🤔

Placing it in the ObjectPropertyModel would mean that it's only properties that can access such a value. Which I think makes the most sense 🤔 There is a few downsides though, and that is the EnumModel for example, don't have access to the values so if we anywhere within the enum rendering want to do something specific with them, that would be impossible. Then there is also the downside that single types such as inputs like this:

{
   "type": ["string", "boolean"]
   "default": "test"
}

In typescript at least could be generated as:

const root: string | boolean = 'test';

But we don't currently do this with the generators as we can only generate:

type root = string | boolean;

which of course cannot have an associated value. I am not sure if we ever will see such an output? Hmm 🤔

What do you think @kennethaasan?

jonaslagoni avatar Oct 14 '22 23:10 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 Feb 15 '23 00:02 github-actions[bot]

I'll get back to this one in March/April

kennethaasan avatar Feb 17 '23 09:02 kennethaasan

:tada: This issue has been resolved in version 2.0.0-next.7 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

asyncapi-bot avatar Apr 19 '23 11:04 asyncapi-bot