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

Allow literal types other than 'string' to be used and properly interpreted as const values #1823

Open
moritzkalwa opened this issue Feb 19, 2024 · 5 comments
Labels
enhancement New feature or request

Comments

@moritzkalwa
Copy link
Contributor

Reason/Context

Please try answering few of those questions

  • Why we need this improvement?
    The behaviour of the const field is inconsistent without a reason (that im aware of)
  • How will this change help?
    The const field will support all literal types.
  • What is the motivation?

Description

Please try answering few of those questions

  • What changes have to be introduced?
    The constant constrainers for all languages need to be adapted to also work with ConstrainedFloatModel ConstrainedIntegerModel and ConstrainedBooleanModel.
  • Will this be a breaking change?
    Yes, the generated models will have different types and const properties will no longer have a setter.
  • How could it be implemented/designed?
    I am not sure yet if we should verify that the const values are valid and fit their type. Would really love some input on this.
@moritzkalwa moritzkalwa added the enhancement New feature or request label Feb 19, 2024
@jonaslagoni
Copy link
Sponsor Member

From the input perspective, you only have valid JS values that it can be: https://www.w3schools.com/js/js_datatypes.asp

From that, you might need to force those values to different language values, i.e. in Java you could have Double(value) or in C# Convert.ToDouble(int) (are you only referring to C# for this issue, or all languages?)

And this is gonna be in conjunction with how each generator do their type mapping

export const CSharpDefaultTypeMapping: CSharpTypeMapping = {

@moritzkalwa
Copy link
Contributor Author

From that, you might need to force those values to different language values, i.e. in Java you could have Double(value) or in C# Convert.ToDouble(int) (are you only referring to C# for this issue, or all languages?)

So you're suggesting ensuring the types at runtime?

I would very much prefer keeping consistency, at least for those generators that already implement the constantConstrainer.

@jonaslagoni
Copy link
Sponsor Member

So you're suggesting ensuring the types at runtime?

Can you clarify?

I would very much prefer keeping consistency, at least for those generators that already implement the constantConstrainer.

Agree 👍

@moritzkalwa
Copy link
Contributor Author

So you're suggesting ensuring the types at runtime?

Can you clarify?

I am honestly just a little confused myself 😅 Those methods are part of Java/C# so we would could use them in our generated models, the real conversion would then only happen at runtime though, right?

@jonaslagoni
Copy link
Sponsor Member

jonaslagoni commented Feb 19, 2024

Yes, for example, the const value might be 1208.2 from the input and the place you render it, you might do Convert.ToDouble(${constValue}) in Modelina which renders as Convert.ToDouble(1208.2) in the model, which of course then becomes at runtime they are converted, yes 🙂

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

No branches or pull requests

2 participants