Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

IDL generated code suggestions [13724] #38

Closed
jwillemsen opened this issue Jul 7, 2020 · 3 comments
Closed

IDL generated code suggestions [13724] #38

jwillemsen opened this issue Jul 7, 2020 · 3 comments
Labels
enhancement New feature or request

Comments

@jwillemsen
Copy link

When looking at the IDL generated code (for example https://github.com/eProsima/Fast-DDS/blob/master/examples/C%2B%2B/Keys/sample.h) I have the following suggestions:

  • Make use of C++11 uniform initialization (see for example) https://www.geeksforgeeks.org/uniform-initialization-in-c/ to initialize all members, that way the constructor can just be default
  • Make use of the default feature for copy/move assignment/constructor, only unions need some special handling, for structs there is nothing special so the compiler default should work
  • isKeyDefined() could maybe be a std::true_type or std::false_type, see https://stackoverflow.com/questions/58694521/what-is-stdfalse-type-or-stdtrue-type and also the IDL to C++11 language mapping uses that for its IDL traits
  • destructor can be default in most cases
  • there are some static methods generated which are fastdds specific, maybe move them to a separate templated struct so they are clearly separated, the user doesn't want to see those when he looks at his idl defined type
  • this example doesn't use any array/vector, so why add includes for that to the header?
  • the class has no explicit constructor accepting values for each struct member in the order they are specified in IDL
  • there is no namespace level swap generated as defined by IDL to C++11
@jwillemsen
Copy link
Author

@jwillemsen
Copy link
Author

jwillemsen commented Jul 7, 2020

#39 is about unions, these can benefit from several of the suggestions in this issue also

@MiguelCompany MiguelCompany transferred this issue from eProsima/Fast-DDS Jul 30, 2020
@MiguelCompany MiguelCompany added the enhancement New feature or request label Jan 14, 2021
@JLBuenoLopez JLBuenoLopez changed the title IDL generated code suggestions IDL generated code suggestions [13724] Feb 1, 2022
@JLBuenoLopez
Copy link
Contributor

Thanks for the suggestions @jwillemsen

We are open to external contributions so feel free to open the corresponding PRs if you want to. I am also moving this ticket to the Ideas discussion forum where enhancements and feature requests are discussed.

@eProsima eProsima locked and limited conversation to collaborators Sep 29, 2023
@JLBuenoLopez JLBuenoLopez converted this issue into discussion #240 Sep 29, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants