modelina
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
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; }
}
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?
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:
I'll get back to this one in March/April
: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: