Open utterances-bot opened 2 years ago
Greetings Kind Regards Thank You for your fine site I utilize std::optional for various reasons e.g. data which can only be initialized after its declaration also as an argument type to support the delegation of function calls from those w/ less data available to them to those w/ more for otherwise identical behavior also for return values of search functions which may not find what they are searching for also for return values of functions which may discover after examining the data they are provided they are unable to provide the service otherwise intended so the caller is spared such examination - Cheerio
Thanks, @PaltryProgrammer for the comment! what have you used before C++17 for those elements you mentioned? have you leveraged Boost or some other libraries?
Greetings Kind Regards In prior work the only problem I encountered from the above list was that of declaring data w/o initialization and I confess I merely left it uninitialized w/ a flag to indicate its state I never have utilized Boost or other libraries other then the STL - Cheerio
That was a nice read! Only thing I'd expect from the final implementation that isn't there, is the ability for the user to give their own default value to the "get_or()" call. Perhaps either two functions: [[nodiscard]] constexpr auto const get_or( ) const noexcept -> T // Which returns default-constructed value if no data [[nodiscard]] constexpr auto const get_or(const T default) const noexcept -> T // Which returns the value given by user if no data
or
[[nodiscard]] constexpr auto const get_or(const T default = T()) const noexcept -> T , which will default-construct the return value if no value is passed.
Perhaps this is just something I've gotten used to with libraries we use at work, just my €0.02.
Great exploration and discussion! Would you consider adding a templated conversion operator from Result
@Pepojovi - Glad you liked the article. When I finally updated my RVR code to use Result
I added the default parameter just as you suggested.
@NibBomb - Thanks for your positive comment. I'd hesitate to add that conversion, at least for the purpose you describe. That conversion eliminates the need for the the class. Why not just return T? Without testing I expect it might cause some issues with return values being automatically converted when that isn't desired.
@Pepejovi & @NikBomb...just to notify you of the comment above where I messed up both your tags.
To be pedantic, you want to use "private" inheritance instead of "protected", here. "protected" allows classes derived from Result to access Result's private members. This is clearly not what you're looking for here. As you note yourself, I would also recommend not inheriting from std classes. I don't have a definitive reason for that, but it looks like a code smell to me.
Finally, if we take at look at your final result (pun intended), it seems to me that Result IS std::optional: it has the same role and works at the same level of abstraction. It only offers slightly different naming (eg: valid() instead of has_value(), get() instead of value(), ...). Thus, if you work with other people, I would recommend to use std::optional directly instead of Result: people already know std::optional, you're doing them a favor by not making them learn another type.
Of course, this means that you end up with zero code. Which is a good thing : no code = no bugs ! Sometimes it's the not the destination, It's the journey.
Brain fart in my comment above, it should read: "protected" allows classes derived from Result to access the std::any base. This is clearly not what you're looking for here.
@jsouquie - OO is about creating abstractions to make writing software easier and understandable. The Result
class is such an abstraction. It provides consistency in handling return values.
@rmerriam true, but I agree with @jsouquie that what you've ended up with is not an abstraction. It's just a renaming of std::optional
with renamed methods, serving the exact same purpose. Which is fine, as you said, it's a learning experience. You have learned along the way that what you needed was actually just the feature set of std::optional
, so you should simply use that.
The only reason I can see to use Result<T>
instead, is that you can change its interface in the future without changing it's name. However, this can simply be achieved by two lines:
template<typename T>
using Result<T> = std::optional<T>;
Although I question whether even this makes sense - there is absolutely an advantage to using well known types with well defined semantics, especially in a library interface. Any user familiar with modern cpp will instantly know how to interpret an std::optional<T>
, but they'll have to look up Result<T>
. It will make your interface less understandable imo.
Beautiful read, Rud.
My comment is regarding constructors. Why do we limit the creation of a Result
with a value to const T&&
? In the case of ResultString
, for example, that would mean we can never move the string. Why not implement all the possible copy/move options so that we benefit from move semantics? E.g.: https://godbolt.org/z/zn11WWTn1
Thanks!
I do totally agree with @jsouquie and @Tuniwutzi; it's ok to experiment, it's a learning experience but IMO Result
A few smells that I see in the Result class:
void buggy(Result b)
{
...
}
// somewhere else in code...
buggy(3); // oops The call to 'buggy(3)' compiles without even a warning and silently converts the integer 3 in Result<3>. Now, I know that probably this is not a use case for the Result class, but it's something to take into account and to be aware of when designing a type
All these things to think about, suddenly disappear by simply using std::optional; someone else has already solved all these (and others) problems for us.
That said, it's always good to experiment and learn.
Sorry, im my previous comment, there's a typo in the example with the function 'buggy'; it was meant to be Result
@StefanoBell Using explicit
for the custom constructor is spot on. The std::move
wouldn't work though, as T&&
is marked const
.
@rturrado you're right. Actually, I cannot think about a use case where it make sense to mark 'const' a T&& (intended as an r-value). That disables the purpose of std::move.. it's a kind of contradiction I think. One more reason to go with the standard library ;)
@StefanoBell, @rturrado, @Tuniwutzi - Wow! You’re a tough crowd. But I appreciate you taking the time to comment, especially @rturrado for the code example.
We’ll have to agree to disagree about whether it is a new abstraction or not. It is an example of the facade pattern: a structural design pattern that provides a simplified interface to a library, a framework, or any other complex set of classes. The class Result
simplifies std::optional
by removing constructors and methods that are not needed for its purpose. Making the default constructor explicit
is to prevent accidental invocations. This seemed more aligned with the special purpose of the class.
The constructors and assignment methods in @rturrado’s code are not needed because std::optional
handles all the details like the forwarding operations.
As I introduced the code to my RVR project I did add constexpr Result(T const &t);
and based on comments changed to constexpr Result(T &&t);
. The previous use of const
was reflex for all parameters. Also removed noexcept
following the usage in std::optional
.
Here’s the new version of the code on Compiler Explorer with a quick unit test function.
I don’t understand the buggy problem nor why constexpr Result(T&& t)
should be explicit.
Quick remark: you forgot the std::move() on t in the constructor taking an rvalue. Like this:
constexpr Result(T &&t) : mOptional{std::move(t)} {}
@rmerriam I didn't meant to be tough, I'm sorry. The reason for having "explicit" the constructor taking the T&& is to avoid that it behaves like a "conversion constructor". A conversion constructor is a constructor that silently converts a type T in your destination type, i.e. in Result< T>. In general, it's a good idea to have explicit constructors, exactly to avoid such implicit conversions, unless you really want you're type to behave that way. The example with the function buggy shows the following things:
What I don't understand instead is when you say you have marked as explicit the default constructor to prevent accidental invocation.... what do you mean? How can you invoke the default constructor accidentally? The explicit keyword should be useless in a default constructor as far as I know, isn't It?
Thanks @rmerriam ! I'm also learning myself, so any time spent testing code is well rewarded :)
I think you still need to propagate the type of t
. Within Result (T&& t)
, even if you receive an rvalue reference, t
is an lvalue reference. If you just do mOptional{ t }
, you'll be passing an lvalue reference to the std::optional
constructor. That will make the std::optional
constructor do a copy instead of a move, which, for large objects, can be relevant. The following code snippet tries to show that: https://godbolt.org/z/9h9oahPE4
And this other code, apart from showing where a copy and a move is done, prints the type of t
and std::forward<T>(t)
as they would be seen by std::optional
constructor: https://godbolt.org/z/59vKWTrTd
As @jsouquie pointed out, you can just use std::move(t)
, instead of std::forward<T>(t)
since Result(T&& t)
will always receive an rvalue reference in your case.
3
to ResultInt{3}
, and forces the programmer to explicitly create instances of Result
: https://godbolt.org/z/za9756zMEstd::move() would be the preferred construction to use here (more idiomatic). std::forward() should typically be used for "forwarding references".
The difference is subtle:
template<typename T>
class Result
{
public:
// Here, T&& other is a rvalue reference, it can bind to rvalues only
// I'm guessing you could use forward<T>() too, but move() is the usual way.
Result( T&& other ) : mValue( std::move(other) ) {}
// Here, U&& other is a forwarding reference because U is a type template _for the method_.
// It can bind to anything.
// We want to use forward to preserve the exact type of the argument
template<typename U>
Result( U&& other ) : mValue( std::forward<U>(other) ) {}
private:
std::optional<T> mValue;
};
See https://en.cppreference.com/w/cpp/utility/move
Names of rvalue reference variables are lvalues and have to be converted to xvalues to be bound to the function overloads that accept rvalue reference parameters, which is why move constructors and move assignment operators typically use std::move: ... One exception is when the type of the function parameter is rvalue reference to type template parameter ("forwarding reference" or "universal reference"), in which case std::forward is used instead.
@jsouquie
The phrase “tough group” is a sly appreciation of comments. It started with comedians who after not getting laughs from their first jokes would comment on the “tough crowd”. No apologies needed.
@rturrado
You are correct. The argument for std::move
hinges on which constructor is called by Result(T&&)
. Is it the T&
or T&&
constructor? Here is code at Compiler Explorer that tests this. When std::move
is not used the T&
constructor is used. With it the T&&
constructor is used. The same is true for operator=(T&&)
. The routine std::move
just does a static_cast
to make the lvalue an rvalue.
From a vague memory I realized the constructors in Derived
can be replaced with using Base<T>::Base;
. This inherits the constructors. The trade off is the template type cannot be deducted in Derived i(3);
. The type must be specified with Derived<int> i(3);
. Since I do using DerivedInt = Derived<int>;
for PODs it might be usable.
Still wrestling with explicit
on the constructors. Tried it in my code base and there are errors but it also isn’t a big hassle to fix them, just a pain.
Thanks for the comments. I think that covers everything. This may be worth turning into an article.
C++ Return: std::any, std::optional, or std::variant? - C++ Stories
What should happen when the data returned from a function is not valid? It might be an error or just how the system operates (embedded environment, a timeout). In this article, you’ll see a practical example from the robotics area where the vocabulary types from C++17 play important roles. This is a guest post written by Rud Merriam:
https://www.cppstories.com/2021/sphero-cpp-return/