Erasing from a Std::Vector While Doing a For Each

Erasing from a std::vector while doing a for each?

erase() returns a new iterator:

for(iterator it = begin; it != end(container) /* !!! */;)
{
if (it->somecondition())
{
it = vec.erase(it); // Returns the new iterator to continue from.
}
else
{
++it;
}
}

Note that we can no longer compare it against a precalculated end, because we may erase it and therefore invalidate it. We must get the end explicitly each time.

A better method might be to combine std::remove_if and erase(). You change from being O(N2) (every element gets erased and shifted as you go) to O(N):

iterator it = std::remove_if(begin, end, pred);
vec.erase(it, vec.end());

Where pred is your removal predicate, such as:

struct predicate // do choose a better name
{
bool operator()(const T& pX) const // replace T with your type
{
return pX.shouldIBeRemoved();
}
};

iterator it = std::remove_if(begin, end, predicate());
vec.erase(it, vec.end());

In your case, you can make it pretty general:

class remove_by_caller
{
public:
remove_by_caller(AguiWidgetBase* pWidget) :
mWidget(pWidget)
{}

// if every thing that has getCaller has a base, use that instead
template <typename T> // for now a template
bool operator()(const T& pX) const
{
return pX.getCaller() == mWidget;
}

private:
AguiWidgetBase* mWidget;
};

std::vector<AguiTimedEvent>::iterator it =
std::remove_if(timedEvents.begin(), timedEvents.end(), remove_by_caller(widget));
timedEvents.erase(it, timedEvents.end());

Note lambda's exist to simplify this process, both in Boost and C++11.

Erase element in vector while iterating the same vector

In the line:

it2 = uc.erase(it2);

an element pointed by iterator it2 is removed from the vector, elements are shifted in memory in order to fill that gap which invalidates it2. it2 gets a new value and now points to the first element after the the removed one or the end of the vector (if removed element was the last one). This means that after erasing an element you should not advance it2. An alternative to proposed remove-erase idiom is a simple trick:

for(it2 = uc.begin(); it2 != uc.end();)
{
...
if(...)
{
it2 = uc.erase(it2);
}
else
{
++it2;
}
...
}

You can read more about this here.

Edit:
Regarding your comment, you can use a flag to pass the information whether an element has been erased or not, and you can check it when you get out from the inner loop:

for(it2=uc.begin(); it2 != uc.end();)
{
bool bErased = false;

for(it3 = c.begin(); it3 != c.end(); ++it3)
{
if(adjacencyMatris[(*it2).id][(*it3).id] == 0 )
{
B.id = (*it2).id;
it2 = uc.erase(it2);
bErased = true;
B.color = currentColor;
c.push_back(B);
break;
}
}

if(!bErased)
++it2;
}

After you've erased an element from uc you need to break from the inner loop. In the next iteration of the outer loop you'll be able to access the next element in the uc through a valid iterator.

Erase some of a vector's elements in a for-each loop without iterating the whole vector

You should still use std::remove_if, just call std::find beforehand.

auto el = std::find(vec.begin(), vec.end(), whatImLookingFor);
auto p = std::remove_if(vec.begin(), el, isInvalid);

// returns the iterator, not the element itself.
// if the element is not found, el will be vec.end()
return vec.erase(p, el);

This will usually be more efficient than removing one element at a time.

Erasing vector elements while in a range-based loop vs. standard loop

In the first case, for each iteration std::vector::size function is being called. Thus, if you delete all the elements in the first iteration, std::vector::size function which is called before the start of second iteration will return 0. Therefore, second iteration won't happen because the condition i < test.size() is not satisfied.

In the second case, range-based for loop uses iterators instead of std::vector::size function. When you call std::vector::erase you invalidate all the iterators including the end() iterator. Therefore, second case is actually UB (Undefined Behavior) and you should never rely on that.

From the docs:

std::vector::erase

... Invalidates iterators and references at or after the point of the
erase, including the end() iterator.

Erasing indices of std::vector inside a for loop based on vector size

Each time you erase() an element from a container, its size() decrements, and the indexes of the remaining elements are decremented as well. But you are incrementing your loop counter unconditionally, so every time you erase an element, you skip the next element that had followed it!

Also, you are passing your vector by-value, so you are operating on a copy of the vector, and the caller will not see any changes in the original vector.

The correct approach would be to either:

  1. increment your index variable inside of the loop body only when an element is NOT erased. Leave the variable as-is when you DO erase an element:

    void FilterContours( std::vector<std::vector<cv::Point>> &contours )
    {
    int i = 0;
    while ( i < contours.size() ) {
    if ( contours[i].size() < 5 ) {
    contours.erase(contours.begin() + i);
    continue;
    }

    //Other filtering...

    ++i;
    }
    }
  2. use iterators instead of indexes:

    void FilterContours( std::vector<std::vector<cv::Point>> &contours )
    {
    auto it = contours.begin();
    while ( it != contours.end() ) {
    if ( it->size() < 5 ) {
    it = contours.erase(it);
    continue;
    }

    //Other filtering...

    ++it;
    }
    }
  3. use the erase-remove idiom:

    void FilterContours( std::vector<std::vector<cv::Point>> &contours )
    {
    contours.erase(
    std:::remove_if(
    contours.begin(),
    contours.end(),
    [](const std::vector<cv::Point> &v)
    {
    if (v.size() < 5) return true;
    //Other filtering...
    return false;
    }
    ),
    contours.end()
    );
    }

How to erase or change element while iterating over vector in C++?

Instead of erasing elements in the middle of the vector, you should write the results from the beginning of the vector and eliminate the unused elements in the end of vector.

int finalSize = 0;
for(int i = 0; i < N; i++)
{
if(result[i] != 0) {
result[finalSize++] = i;
}
}
result.resize(finalSize);

Removing item from vector, while in C++11 range 'for' loop?

No, you can't. Range-based for is for when you need to access each element of a container once.

You should use the normal for loop or one of its cousins if you need to modify the container as you go along, access an element more than once, or otherwise iterate in a non-linear fashion through the container.

For example:

auto i = std::begin(inv);

while (i != std::end(inv)) {
// Do some stuff
if (blah)
i = inv.erase(i);
else
++i;
}

How to delete an element from a vector while looping over it?

The idiomatic way to remove all elements from an STL container which satisfy a given predicate is to use the remove-erase idiom. The idea is to move the predicate (that's the function which yields true or false for some element) into a given function, say pred and then:

static bool pred( const std::string &s ) {
// ...
}

std::vector<std::string> v;
v.erase( std::remove_if( v.begin(), v.end(), pred ), v.end() );

If you insist on using indices, you should not increment the index for every element, but only for those which didn't get removed:

std::vector<std::string>::size_type i = 0;
while ( i < v.size() ) {
if ( shouldBeRemoved( v[i] ) ) {
v.erase( v.begin() + i );
} else {
++i;
}
}

However, this is not only more code and less idiomatic (read: C++ programmers actually have to look at the code whereas the 'erase & remove' idiom immediately gives some idea what's going on), but also much less efficient because vectors store their elements in one contiguous block of memory, so erasing on positions other than the vector end also moves all the elements after the segment erased to their new positions.



Related Topics



Leave a reply



Submit