Write `for (x: y) if (p (x)) returns x;` in modern algorithmic form
I always try to use STL-like algorithms whenever possible as they are short and very expressive.
I have this code in one of my libraries:
auto& findFlag(const std::string& mName)
{
for(auto& f : makeRangeCastRef<Flag>(getFlags()))
if(f.hasName(mName))
return f;
throw Exception::createFlagNotFound(mName, getNamesStr());
}
I would like to write it in modern algorithmic C ++ form, but I cannot figure out how to deal with early return
and possible throw
.
for(auto& value : container) if(predicate(value)) return value;
// ^~~~~~
// IMPORTANT: return from the caller function, not the algorithm itself
Ideally, I would like to write a real piece of code as:
auto& findFlag(const std::string& mName)
{
early_return_if(makeRangeCastRef<Flag>(getFlags()),
[&mName](const auto& f){ return f.hasName(mName); });
throw Exception::createFlagNotFound(mName, getNamesStr());
}
Obviously something like early_return_if
can't exist - there is no way to call return
the caller function from the callee in my opinion . return early_return_if(...)
might work, but then I cannot throw an exception without creating a specific algorithm that throws exceptions.
What do you suggest? Should the code be kept as it is, or is there some algorithmic way I can rewrite it?
EDIT:
As pointed out in the comments, std::find_if
is a good candidate, but there is unnecessary validation that can be avoided:
auto& findFlag(const std::string& mName)
{
auto container(makeRangeCastRef<Flag>(getFlags())); // Need to write this out...
// Need to keep track of the returned iterator...
auto it(findIf(container, [&mName](const auto& f){ return f.hasName(mName); }));
if(it != container.end()) return *it; // I don't like this either...
throw Exception::createFlagNotFound(mName, getNamesStr());
}
source to share
The range based algorithm it uses boost::optional
. reference_type_t
left as an exercise (hint: write iterator_type_t
first based on adl begin
range search ).
template<class Range, class Function>
boost::optional< reference_type_t<Range> >
search_if( Range&& r, Function&& f ) {
for( auto&& x:std::forward<Range>(r) ) {
if (f(x))
return std::forward<decltype(x)>(x);
}
return {};
}
Then:
auto& findFlag(const std::string& mName) {
auto result = search_if(
makeRangeCastRef<Flag>(getFlags()),
[&](auto&& f){return f.hasName(mName); }
);
if (result) return *result;
throw Exception::createFlagNotFound(mName, getNamesStr());
}
you can throw the exception entirely and findFlag
return optional
yourself (which basically does it search_if
).
And no, you cannot inject flow control into a function that calls you.
The above relies on optional
which supports optional links. They are inconsistent: the upstream std::optional
did not support them the last time I checked.
You can also replace this optional
with simple T*
s.
template<class Range, class Function>
value_type_t<Range>*
search_if( Range&& r, Function&& f ) {
for( auto&& x:std::forward<Range>(r) ) {
if (f(x))
return &x;
}
return nullptr;
}
but the downside is that if your range is weird (for example std::vector<bool>
), you will get the temp link above.
A sketch value_type_t
and reference_type_t
that takes a range / container and outputs the value / type / value type of range / container:
namespace adl_aux {
using std::begin;
template<class R> using iterator_t = decltype( begin(std::declval<R>()) );
}
using adl_aux iterator_t;
template<class T>struct void{using type=void;}
template<class T>using void_t=typename void<T>::type;
template<class R,class=void>
struct value_type {};
template<class R>
struct value_type<R, void_t< iterator_t<R> > {
using type = std::iterator_traits< iterator_t<R> >::value_type;
};
template<class R>using value_type_t = typename value_type<R>::type;
template<class R,class=void>
struct reference_type {};
template<class R>
struct reference_type<R, void_t< iterator_t<R> > {
using type = std::iterator_traits< iterator_t<R> >::reference_type;
};
template<class R>using reference_type_t = typename reference_type<R>::type;
it can be made more reliable - checking SFINAE on iterators can check the axioms of the iterator on the return type begin
and ensure that it end
is either an identical iterator or compatible from there.
source to share
I'm not sure what exactly makeRangeCastRef <> () and some of your other code do, but personally I think the find_if version is more readable than your original version if you write it more like this:
auto& findFlag(const std::string& mName)
{
auto findIt = find_if(cbegin(getFlags()), cend(getFlags()), [&](const auto& f){ return f.hasName(mName); });
if (findIt == container.end()) throw Exception::createFlagNotFound(mName, getNamesStr());
return *findIt;
}
It seems more natural to me to check for an exceptional condition (flag not found) and throw an exception, otherwise fall through to the normal exit path of the returned found element, rather than to a loop-based version returning from within the loop in a "normal" state and fail otherwise. to throw an exception.
source to share
Using Alexandrescu Expected<T>
, you can write an algorithm that returns an object that converts to the item you are looking for, or throws an exception if it was not found. Something like (didn't compile this):
template <class It, class Pred, class Else>
Expexted<T&> find_if_ref(It first, It last, Pred pred, Else el)
{
auto it = find_if(first, last, pred);
if (it == last) {
try {
el();
}
catch (...) {
return std::current_exception();
}
}
return *it;
}
source to share