-
Notifications
You must be signed in to change notification settings - Fork 39
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 Ref class to GeneriContainer::Vector #102
Add Ref class to GeneriContainer::Vector #102
Conversation
The ambiguity is due to the fact that a non-const object can be used both with the const and non-const version of foo.
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.
That's cool, @S-Dafarra!
So thanks to this PR we can change
https://github.com/dic-iit/bipedal-locomotion-framework/blob/6923a1fba872796d6143e7a3bf735a49df8e194c/src/ParametersHandler/include/BipedalLocomotion/ParametersHandler/IParametersHandler.h#L35
into
virtual bool getParameter(const std::string& parameterName, GenericContainer::Vector<int>::Ref parameter) const = 0
right?
Yes, we can get rid also of the template method, and move all those using a |
I agree, feel free to merge it |
template <class Vector, typename = typename std::enable_if<!GenericContainer::is_vector<Vector>::value && | ||
!std::is_same<Vector, std::string>::value && |
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.
in C++17 we have is_same_v
which can replace is_same<T, T>::value
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.
Yes, it is a little more verbose in this way, but some static analyzers complain less sometimes 😁
template <class Vector, typename = typename std::enable_if<!GenericContainer::is_vector<Vector>::value && | ||
!std::is_same<Vector, std::string>::value && | ||
GenericContainer::is_vector_constructible<Vector>::value && | ||
std::is_const_v<T>>::type> |
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.
Just a curiosity. Can this be SFINAE construct be simplified using an if constexpr
in this specific scenario? If not, could you tell me why?
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.
SFINAE makes the function/method below disappear. This allows the compiler to choose one version or the other. if constexpr
instead is used inside the function. Hence, you can use it to compile a method in two different ways while maintaining the same signature. Rule of thumb:
- use SFINAE to drive the compiler toward the correct method, when different methods have ambiguous signatures
- use
if constexpr
to conditionally compile some pieces of code inside a method depending on some condition known at compile time.
With c++20
SFINAE may be substituted with a concept
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.
Understood! Thanks for the pointers!
The class
Ref
is used as a substitution to a classical reference to a Vector. The advantage of using this, is that custom vectors (all those supported byGenericContainer::Vector
) can be implicitly casted toRef
.Ref
does not allocate any memory in construction, hence can be used as a parameter to be passed by copy. The operator=
clones the content.Ref
inheritsVector<T>
, hence it can be used as it was aVector<T>
.This goes in the direction of fixing #95, at least to simplify the use of
GenericContainer::Vector<double>
in public interfaces.