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

Add support for records with the C# generator through an option #1153

Closed
henrikjon opened this issue Mar 13, 2023 · 7 comments · Fixed by #1191
Closed

Add support for records with the C# generator through an option #1153

henrikjon opened this issue Mar 13, 2023 · 7 comments · Fixed by #1191
Labels

Comments

@henrikjon
Copy link
Contributor

Reason/Context

Please try answering few of those questions

  • Why we need this improvement?
    • Records is a popular way of defining DTO's in C#
  • How will this change help?
    • It gives more flexibility for C# users
  • What is the motivation?
    • Records are a great match for DTO's in C#:
      • Reference based records are immutable by default using short declaration, ex: public record Car(int Doors, string Manufacturer);
      • Made for containing data, not for logic
      • They use value based comparison

Description

Please try answering few of those questions

  • What changes have to be introduced?
  • Will this be a breaking change?
  • How could it be implemented/designed?
@henrikjon henrikjon added the enhancement New feature or request label Mar 13, 2023
@github-actions
Copy link
Contributor

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.

@jonaslagoni
Copy link
Member

After discussing this in slack, we think the best approach is to do a similar thing we did for TypeScript and interface/class, where we conditionally render one or the other.

So to achieve this in the C# generator we need to do the following changes:

  1. We add a model type option as typescript which is either class by default or record.
  2. Create a new RecordRenderer, you can use the class renderer as starting point.
  3. The new renderer should have the following hooks: self, property, additionalContent
  4. self should render the full syntax, not the shorthand one, for the simple reason its easier to add custom functions and handle in general with no penalty. So the hook should return:
public record ${model.name}
{
  ${render all properties}
  ${render additionalContent}
};

so it should look like this:

public record Person
{
    public required string FirstName { get; set; }
    public required string LastName { get; set; }
};
  1. property should return public ${property.required ? 'required ' : ' '}${property.type} ${property.name} { ${render getter} ${render setter} } so it assembles public required string FirstName { get; set; }
  2. getter should just return get;
  3. setter should just return init;
  4. additionalContent should not return anything.
  5. Change the generator to conditionally use the record renderer instead of class renderer based on the type option added in step 1.

How does that sound to you @henrikjon?

@henrikjon
Copy link
Contributor Author

Sounds really good i think, and your description on what to do is clear so it should be pretty straight forward to implement. Ill be happy to have a go at it later this week @jonaslagoni.

@jonaslagoni
Copy link
Member

Awesome, looking forward to it @henrikjon!

Let me know if I can help with anything 🙂

@henrikjon
Copy link
Contributor Author

@jonaslagoni thanks! Thats very generous, I have working code with a snapshot test but might still have a couple of things to iron out that i have not discovered yet. Trying to get a bit more familiar with the codebase before i submit the pull request.

I did however create a local draft in my fork: henrikjon#1 . All feedback is very welcome :)

@asyncapi-bot
Copy link
Contributor

🎉 This issue has been resolved in version 1.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@asyncapi-bot
Copy link
Contributor

🎉 This issue has been resolved in version 2.0.0-next.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants