[std-proposals] Bikeshedding P1223R0, find_backward() etc.

[k***@gmail.com]

Hi,

P1223R0 proposes to add various overloads of these new functions to
<algorithm> to operate upon bidirectional iterators or ranges composed
thereof:

find_backward(), find_not_backward(), find_if_backward(),
find_if_not_backward(), find_not().

I have two suggestions:

First, there is no reason why these functions shouldn't have a
specialization that produces the obvious result when operating on a forward
iterator range (that is not bidirectional). E.g. a possible implementation
for this specialization of find_backward() that returns an iterator
addressing the *last* element comparing equal to the search value if any
are present, otherwise the end-of-range:

template <typename ForwardIterator, typename Sentinel, typename T>
auto find_backward(ForwardIterator begin, Sentinel end, const T & val)
{
ForwardIterator result = end;
while (begin != end) {
if (val == *begin)
result = begin;
++begin;
}
return result;
}

Second: respectfully I suggest that for brevity (as well as for symmetry
with std::basic_string member functions), std::find_backward() might be
named std::rfind() instead. This would also fix any misleading impression
that the above-suggested forward iterator specialization was somehow
traversing the range backward.

Perfect symmetry with std::basic_string is probably not desirable, since it
would imply that find_not_backward() ought to be given the terribly clunky
name find_last_not_of(), and similar. I would suggest instead:

find_backward --> rfind
find_not_backward --> rfind_not
find_if_backward --> rfind_if
find_if_not_backward --> rfind_if_not
[find_not remains as-is]

Thanks for considering,

[m***@gmail.com]

I personally am against the original proposal as it does not scale - we
must add _backwards versions to all similar algorithms OR unexperienced new
user will look for them, baffled why there is no upper_bound_backwards or
something.

The user must learn about reverse iterators and the mix-and-match nature of
the STL and that he is free to create new functions by combinations of the
core algorithms and giving them names he wants.

That said I agree it is a glaring naming issue that

auto rfirst = std::make_reverse_iterator(it);
auto rlast = std::make_reverse_iterator(first);
auto it = std::find(rfirst, rlast, x);

will be the same as

auto it = std::find_backward(first, it, x);

How can one teach that?


Also, the original proposal ignores the fact make_ is no longer needed

auto it = std::find(std::reverse_iterator(it),
std::reverse_iterator(first), x);

Now, I believe this is more clear then std::find_backward(first, it, x); as
the starting iterator is it and the last iterator is first - this maps to
the mental image, we are traversing from the back to the front

It might be subjective but I expect first to be the first in the direction
I am traversing not first in the collection/memory!

Post by k***@gmail.com
Hi,
P1223R0 proposes to add various overloads of these new functions to
<algorithm> to operate upon bidirectional iterators or ranges composed
find_backward(), find_not_backward(), find_if_backward(),
find_if_not_backward(), find_not().
First, there is no reason why these functions shouldn't have a
specialization that produces the obvious result when operating on a forward
iterator range (that is not bidirectional). E.g. a possible implementation
for this specialization of find_backward() that returns an iterator
addressing the *last* element comparing equal to the search value if any
template <typename ForwardIterator, typename Sentinel, typename T>
auto find_backward(ForwardIterator begin, Sentinel end, const T & val)
{
ForwardIterator result = end;
while (begin != end) {
if (val == *begin)
result = begin;
++begin;
}
return result;
}
Second: respectfully I suggest that for brevity (as well as for symmetry
with std::basic_string member functions), std::find_backward() might be
named std::rfind() instead. This would also fix any misleading impression
that the above-suggested forward iterator specialization was somehow
traversing the range backward.
Perfect symmetry with std::basic_string is probably not desirable, since
it would imply that find_not_backward() ought to be given the terribly
find_backward --> rfind
find_not_backward --> rfind_not
find_if_backward --> rfind_if
find_if_not_backward --> rfind_if_not
[find_not remains as-is]
Thanks for considering,
--
Kevin B. McCarty