-
Notifications
You must be signed in to change notification settings - Fork 193
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 article about generated C++ interfaces #33
Conversation
Each setter methods returns the struct itself to enable (method chaining)[https://isocpp.org/wiki/faq/ctors#named-parameter-idiom}. | ||
|
||
<div class="alert alert-warning" markdown="1"> | ||
<b>TODO:</b> Need non-colliding naming |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a set__
suffix seems like that most readable and non-colliding name to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did that already in 4d4b712 but I am not sure I like it.
327b9e9
to
ec9a06a
Compare
I have update the article based on the discussion in the meeting and removed all references to copy / move constructors / assignment operators. Please notice the change of the pointer types. @ros2/owners Please review. |
+1 |
1 similar comment
+1 |
* `Ptr` and `ConstPtr` | ||
* `SharedPtr` and `ConstSharedPtr` | ||
* `UniquePtr` and `ConstUniquePtr` | ||
* `WeakPtr` and `ConstWeakPtr` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also add auto_ptr
here? Or should we remove any of the above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say no; auto_ptr
is deprecated in C++11. The others look fine. People might find it weird that Ptr
is now a "raw" pointer rather than a shared pointer, as it is in ROS 1. Not sure what else we could do though. Maybe RawPtr
and ConstRawPtr
, but I don't really like that either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-1 for RawPtr
. The definitions in the document for Ptr
types looks fine to me.
+1 |
ec9a06a
to
0ed7369
Compare
add article about generated C++ interfaces
No description provided.