etl
etl copied to clipboard
etl::remove_if calls predicate twice for first iterator, where predicate returns true.
See code below. etl::find_if returns the first iterator, where predicate returns true. So if first != last, predicate is already called for *first. However, itr = first and if (!predicate(*itr)) calls predicate again for *first.
In our code, we have logging for when an item in the iterator has been marked for removal. Resulting in logging this twice for the first element that is marked for removal.
/// Remove If
///\ingroup algorithm
//***************************************************************************
template <typename TIterator, typename TUnaryPredicate>
ETL_CONSTEXPR14
TIterator remove_if(TIterator first, TIterator last, TUnaryPredicate predicate)
{
first = etl::find_if(first, last, predicate);
if (first != last)
{
TIterator itr = first;
while (itr != last)
{
if (!predicate(*itr))
{
*first = etl::move(*itr);
++first;
}
++itr;
}
}
return first;
}
It seems the algorithm works fine without the initial find_if.
@jwellbelove Yes, the algorithm works without the find if, however in case the predicate has a small chance of returning true (i.e. you have a lot of items and want to erase only a small amount) it causes a lot of unnecessary moves, especially if you are trying to remove only items close to the end of collection.
Please consider using something similar to the possible implementation on cppreference: https://en.cppreference.com/w/cpp/algorithm/remove
Notice the clever trick with ++i in the for loop comparison.
Thanks, I'll take a look at that.
I went back to the original implementation and just moved the increment.
first = etl::find_if(first, last, predicate);
if (first != last)
{
TIterator itr = first;
while (++itr != last)
{
if (!predicate(*itr))
{
*first++ = etl::move(*itr);
}
}
}
return first;
Fixed 20.38.12