Should boost have ofind(_if), that returns boost::optional<T&>?
Hi, I have been thinking for this for a long time so I wanted to get the feedback from others. As you probably know better than me there is a bit of a drama <https://brevzin.github.io/c++/2021/12/13/optional-ref-ptr/>when it comes to std::optional<T&> and from what I see it will not be in C++23 for sure. I believe this gives an opportunity to boost to provide something nice for the users. Now the issues are the following: AFAIK boost only has old ranges and iterator based algorithms, ideally we would just have something like std:: ranges and add the ofind/ofind_if to those, but AFAIK those do not exist. So I think best thing would be to add it to boost::algorithm <https://github.com/boostorg/algorithm/tree/develop/include/boost/algorithm> . There are further questions like, should boost containers like map/unordered_map gain similar functionality, but I think deciding if we want the general algorithm is the first step. Another option I can think of is that those algorithms return something like a 0 or 1 element view(so you could access the optional value with range based for loop). This seems weird to me, so I would prefer boost::optional<T&>. There is s a short example on godbolt <https://godbolt.org/z/W9K3na9Gc>, but don't look at implementation too much, it was just something I hacked quickly, I am mostly interested in in feedback on if functionality would be nice addition to boost. regards, Ivan
On Mar 22, 2022, at 2:09 PM, Ivan Matek via Boost <boost@lists.boost.org> wrote:
Hi, I have been thinking for this for a long time so I wanted to get the feedback from others.
As you probably know better than me there is a bit of a drama <https://brevzin.github.io/c++/2021/12/13/optional-ref-ptr/>when it comes to std::optional<T&> and from what I see it will not be in C++23 for sure. I believe this gives an opportunity to boost to provide something nice for the users.
Now the issues are the following: AFAIK boost only has old ranges and iterator based algorithms, ideally we would just have something like std:: ranges and add the ofind/ofind_if to those, but AFAIK those do not exist. So I think best thing would be to add it to boost::algorithm <https://github.com/boostorg/algorithm/tree/develop/include/boost/algorithm> . There are further questions like, should boost containers like map/unordered_map gain similar functionality, but I think deciding if we want the general algorithm is the first step.
Another option I can think of is that those algorithms return something like a 0 or 1 element view(so you could access the optional value with range based for loop). This seems weird to me, so I would prefer boost::optional<T&>.
There is s a short example on godbolt <https://godbolt.org/z/W9K3na9Gc>, but don't look at implementation too much, it was just something I hacked quickly, I am mostly interested in in feedback on if functionality would be nice addition to boost.
Personally, I’m not really that enthusiastic about this interface. However, if someone wants to make a PR for boost.Algorithm, I will certainly review it. — Marshall
Why not return a pointer/nullptr instead? pfind_if Den sön 3 apr. 2022 21:46Marshall Clow via Boost <boost@lists.boost.org> skrev:
On Mar 22, 2022, at 2:09 PM, Ivan Matek via Boost <boost@lists.boost.org> wrote:
Hi, I have been thinking for this for a long time so I wanted to get the feedback from others.
As you probably know better than me there is a bit of a drama <https://brevzin.github.io/c++/2021/12/13/optional-ref-ptr/>when it
comes
to std::optional<T&> and from what I see it will not be in C++23 for sure. I believe this gives an opportunity to boost to provide something nice for the users.
Now the issues are the following: AFAIK boost only has old ranges and iterator based algorithms, ideally we would just have something like std:: ranges and add the ofind/ofind_if to those, but AFAIK those do not exist. So I think best thing would be to add it to boost::algorithm < https://github.com/boostorg/algorithm/tree/develop/include/boost/algorithm
. There are further questions like, should boost containers like map/unordered_map gain similar functionality, but I think deciding if we want the general algorithm is the first step.
Another option I can think of is that those algorithms return something like a 0 or 1 element view(so you could access the optional value with range based for loop). This seems weird to me, so I would prefer boost::optional<T&>.
There is s a short example on godbolt <https://godbolt.org/z/W9K3na9Gc>, but don't look at implementation too much, it was just something I hacked quickly, I am mostly interested in in feedback on if functionality would be nice addition to boost.
Personally, I’m not really that enthusiastic about this interface. However, if someone wants to make a PR for boost.Algorithm, I will certainly review it.
— Marshall
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Apr 3, 2022, at 3:09 PM, Viktor Sehr <viktor.sehr@gmail.com <mailto:viktor.sehr@gmail.com>> wrote:
Why not return a pointer/nullptr instead? pfind_if
Why not just check to see if the returned iterator == end () ? — Marshall
Den sön 3 apr. 2022 21:46Marshall Clow via Boost <boost@lists.boost.org <mailto:boost@lists.boost.org>> skrev: On Mar 22, 2022, at 2:09 PM, Ivan Matek via Boost <boost@lists.boost.org <mailto:boost@lists.boost.org>> wrote:
Hi, I have been thinking for this for a long time so I wanted to get the feedback from others.
As you probably know better than me there is a bit of a drama <https://brevzin.github.io/c++/2021/12/13/optional-ref-ptr/ <https://brevzin.github.io/c++/2021/12/13/optional-ref-ptr/>>when it comes to std::optional<T&> and from what I see it will not be in C++23 for sure. I believe this gives an opportunity to boost to provide something nice for the users.
Now the issues are the following: AFAIK boost only has old ranges and iterator based algorithms, ideally we would just have something like std:: ranges and add the ofind/ofind_if to those, but AFAIK those do not exist. So I think best thing would be to add it to boost::algorithm <https://github.com/boostorg/algorithm/tree/develop/include/boost/algorithm <https://github.com/boostorg/algorithm/tree/develop/include/boost/algorithm>> . There are further questions like, should boost containers like map/unordered_map gain similar functionality, but I think deciding if we want the general algorithm is the first step.
Another option I can think of is that those algorithms return something like a 0 or 1 element view(so you could access the optional value with range based for loop). This seems weird to me, so I would prefer boost::optional<T&>.
There is s a short example on godbolt <https://godbolt.org/z/W9K3na9Gc <https://godbolt.org/z/W9K3na9Gc>>, but don't look at implementation too much, it was just something I hacked quickly, I am mostly interested in in feedback on if functionality would be nice addition to boost.
Personally, I’m not really that enthusiastic about this interface. However, if someone wants to make a PR for boost.Algorithm, I will certainly review it.
— Marshall
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost <http://lists.boost.org/mailman/listinfo.cgi/boost>
On 4/4/22 01:58, Marshall Clow via Boost wrote:
On Apr 3, 2022, at 3:09 PM, Viktor Sehr <viktor.sehr@gmail.com <mailto:viktor.sehr@gmail.com>> wrote:
Why not return a pointer/nullptr instead? pfind_if
Why not just check to see if the returned iterator == end () ?
Exactly. The iterator has the added benefit that it can be used with the container to perform further operations (e.g. remove the element). The only benefit of pointer/optional is that is allows to decouple the element from the container (e.g. if you want to hide the container type from the caller), and this can be achieved with a trivial wrapper function.
On 4/04/2022 10:58, Marshall Clow wrote:
On Apr 3, 2022, at 3:09 PM, Viktor Sehr wrote:
Why not return a pointer/nullptr instead? pfind_if
Why not just check to see if the returned iterator == end () ?
The annoyance with the iterator == end() implementation is that it's a double indirection -- if you want to access the value afterwards it has to be in a separate statement, which means you can't do it with an rvalue collection. The same applies with a pointer-find; it's still a non-owning indirection that's invalid on rvalue collections. The optional return doesn't have that problem; because it copies the value, the value remains valid when returned from an rvalue collection. Granted, rvalue collections can be inefficient in themselves (since it means you're recalculating some collection instead of caching it), but simply making the code unsafe unless you assign the collection to a variable doesn't actually help eliminate that usage, it just makes it wordier. And with the rise of ranges and collection filters/views (which tend to be light rvalues), that sort of coding style will become more common, not less. At the end of the day, which usage makes more sense depends on how you're interacting with the collection -- do you only care about reading the value, or do you need to modify/remove the value? Do you need the value itself or do you only care if it's in there? Is the value cheap to copy? The stdlib interface is deliberately minimal (and rightly so) -- it does *let* you do everything, but it's not the most friendly. (I usually end up making set::contains and map::get helper methods to improve code conciseness and readability; these are the most common "missing" operations.)
Mere moments ago, quoth I:
On 4/04/2022 10:58, Marshall Clow wrote:
On Apr 3, 2022, at 3:09 PM, Viktor Sehr wrote:
Why not return a pointer/nullptr instead? pfind_if
Why not just check to see if the returned iterator == end () ?
The annoyance with the iterator == end() implementation is that it's a double indirection -- if you want to access the value afterwards it has to be in a separate statement, which means you can't do it with an rvalue collection.
To be clearer, it's just the need to access end() again that requires putting the collection in a variable (assuming that you're using a member or range algorithm, or something that otherwise doesn't already require passing separate begin/end iterators).
The optional return doesn't have that problem; because it copies the value, the value remains valid when returned from an rvalue collection.
And this applies only for optional<T>, not optional<T&> as was being suggested. So a different case that's also not going to be valid for rvalue collections.
On Apr 3, 2022, at 5:01 PM, Gavin Lambert via Boost <boost@lists.boost.org> wrote:
On 4/04/2022 10:58, Marshall Clow wrote:
On Apr 3, 2022, at 3:09 PM, Viktor Sehr wrote:
Why not return a pointer/nullptr instead? pfind_if
Why not just check to see if the returned iterator == end () ?
The annoyance with the iterator == end() implementation is that it's a double indirection -- if you want to access the value afterwards it has to be in a separate statement, which means you can't do it with an rvalue collection.
The same applies with a pointer-find; it's still a non-owning indirection that's invalid on rvalue collections.
The optional return doesn't have that problem; because it copies the value, the value remains valid when returned from an rvalue collection.
Actually, that’s not what Ivan proposed. He proposed returning an optional<T&>. — Marshall
On 4/4/22 03:01, Gavin Lambert via Boost wrote:
On 4/04/2022 10:58, Marshall Clow wrote:
On Apr 3, 2022, at 3:09 PM, Viktor Sehr wrote:
Why not return a pointer/nullptr instead? pfind_if
Why not just check to see if the returned iterator == end () ?
The annoyance with the iterator == end() implementation is that it's a double indirection -- if you want to access the value afterwards it has to be in a separate statement, which means you can't do it with an rvalue collection.
I don't see creating a variable as that much of a problem. Besides, C++17 adds support for variable declarations in the `if` statement.
On Mon, Apr 4, 2022 at 12:59 AM Marshall Clow via Boost < boost@lists.boost.org> wrote:
Why not just check to see if the returned iterator == end () ?
Philosophical answer:
because find/find_if algorithm already knew that, he was just rude ;) and did not give us back that information. Practical answer: I find writing !=.end spammy and potentially errorprone. I know it sounds impossible to mess this up but in code that is not slideware mistakes can happen. Also I prefer shorter code(when it comes to syntax spam, not variable names) since for me it makes the logic more obvious. Bonus :) answer: Rust does it. https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.find Ben Deane used it in a talk: https://www.youtube.com/watch?v=ojZbFIQSdl8&t=2138s @Viktor I prefer to avoid pointers since they can be deleted and then bad things can happen. But optional<T&> has similar functionality, it is just harder to mistake for a something you need to delete... Also you can read the Barry blog post on why he prefers optional<T&> over T*, here is picture <https://brevzin.github.io/assets/optional-vs-ptr.png>he used to show they are not really same.... @Andrey Semashev <andrey.semashev@gmail.com> I almost never do anything with the iterator except comparing it to the end(). Again it is not that I use find_if every day, but I use it a lot and I honestly do not remember that I ever wanted to do something further with the iterator. Actually last time I did something with iterators returned from algorithm it was a return value of upper_bound, not find/find_if. So IDK what else to say, I was really thinking this API is much better, especially if Marshall would be interested in allowing container overloads(on compilers with concepts) so code would look like: if (const auto maybe_val = ofind(v, x); maybe_val) { fmt::print("{}\n", *maybe_val); } Beside fact that let or cauto would be much nicer than const auto I think that code looks actually quite nice.
participants (5)
-
Andrey Semashev -
Gavin Lambert -
Ivan Matek -
Marshall Clow -
Viktor Sehr