Remove Elements of a Vector Inside the Loop

Remove elements of a vector inside the loop

You should not increment it in the for loop:

for (vector<Player>::iterator it=allPlayers.begin(); 
                              it!=allPlayers.end(); 
                              /*it++*/) <----------- I commented it.
{

   if(it->getpMoney()<=0) 
      it = allPlayers.erase(it);
  else 
      ++it;
 }

Notice the commented part;it++ is not needed there, as it is getting incremented in the for-body itself.

As for the error "'operator =' function is unavailable in 'Player’", it comes from the usage of erase() which internally uses operator= to move elements in the vector. In order to use erase(), the objects of class Player must be assignable, which means you need to implement operator= for Player class.

Anyway, you should avoid raw loop¹ as much as possible and should prefer to use algorithms instead. In this case, the popular Erase-Remove Idiom can simplify what you're doing.

allPlayers.erase(
    std::remove_if(
        allPlayers.begin(), 
        allPlayers.end(),
        [](Player const & p) { return p.getpMoney() <= 0; }
    ), 
    allPlayers.end()
); 

^{1. It's one of the best talks by Sean Parent that I've ever watched.}

call a function, inside a for loop, to remove elements in a vector (or list)

Erasing while iterating is possible if you increment the iterator inside the loop depending on if you erase the element or not. If you erase it, the erase method returns the next iterator in the vector, if not, increment it yourself. Moreover, you need to initialize the iterator to begin.

vector<node>::iterator it;
for(it=vector_of_node.begin();it!=vector_of_node.end();){
    if(it->get_name()=="MARIA")
        it = vector_of_node.erase(it);
    else
        ++it;
}

how to erase from vector in range-based loop?

You can't erase elements by value on a std::vector, and since range-based loop expose directly values your code doesn't make sense (vec.erase(&i)).

The main problem is that a std::vector invalidates its iterators when you erase an element.

So since the range-based loop is basically implemented as

auto begin = vec.begin();
auto end = vec.end()
for (auto it = begin; it != end; ++it) {
  ..
}

Then erasing a value would invalidate it and break the successive iterations.

If you really want to remove an element while iterating you must take care of updating the iterator correctly:

for (auto it = vec.begin(); it != vec.end(); /* NOTHING */)
{
  if ((*it) > 5)
    it = vec.erase(it);
  else
    ++it;
}  

Remove elements of vector in a loop based on index

Rust is saving you from iterator invalidation (a common source of bugs in other languages). This is an error that usually happens when you try to modify a data structure while concurrently iterating over it. You cannot move on to the (now-deleted) next element after calling element.drain(index..). So you need to add a break after that point to avoid memory unsafety.

In this case just adding break; is sufficient to make the code compile. However, for a more concise, linear solution, take full advantage of the iterators and methods provided by the standard library:

if let Some(index) = element.windows(2).position(|pair| pair[0] == pair[1]) {
    element.truncate(index);
}

windows(2) on a slice gives an iterator over subslices of length 2, and the position call returns the index of the first element of that iterator for which the two elements of the slice are equal. (If no such pair exists, position returns None.)

I find that the position closure becomes more obvious with the (currently unstable) array_windows feature:

if let Some(index) = element.array_windows().position(|[x, y]| x == y) {
    element.truncate(index);
}

Playground

Related

• is it possible to filter on a vector in-place?

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.

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;
}

---

Related Topics