c++ - Keeping a valid vector::iterator after erase()

Question

EDIT: I have had a lot of answers telling me that I should separate the deletion into another loop. Perhaps I did not make it clear enough, but I stated in my last paragraph that I'd like to find a solution OTHER than that. ie keeping the current code structure, but using some little-known C++fu to make it work.

Well, I know that calling erase() on a vector invalidates iterators to the element and all those after it, and that erase() returns an iterator to the next valid iterator, but what if the erase happens elsewhere?

I have the following situation (simplified):

WARNING: Do NOT assume that this is the entire code. What is shown below is EXTREMELY simplified to illustrate my problem. All the classes and methods shown below are actually far more complex.

class Child {
   Parent *parent;
}

class Parent {
   vector<Child*> child;
}

void Parent::erase(Child* a) {
   // find an iterator, it, that points to Child* a
   child.erase(it);
}

int Child::update() {
   if(x()) parent.erase(*this) // Sometimes it will; sometimes (most) it won't
   return y;
}

void Parent::update() {
   int i = 0;
   for(vector<A>::iterator it = child.begin(); it != child.end(); it++)
      i += (*it)->update();
}

So, obviously, it will crash after it runs (*it)->update() if x() returns true, because when it does, the Child will tell the Parent to remove it from the vector, invalidating the iterator.

Is there any way of fixing this other than making Parent::erase() pass an iterator all the way back up to Parent::update()? This would be problematic, as it is not called for every call to Child::update(), and thus that function would need a way to return an iterator to itself every single other time, and it is also currently returning another value. I would also prefer to avoid some other similar way to separate the erasing the process from the updating loop.

Answer 1

I recommend that you restructure your code to not mix the two different actions of updating (by deleting certain elements) and aggregating (by adding up the values) the data.

You could do this by changing the return-value of Child::update to something like std::pair<int, bool>, where the int is the value and the bool indicates if this element should be deleted.

If you can make Child::update a const method (meaning it does not modify the object, and only calls other const methods), you could write a simple functor that you can use with std::remove_if. Something like this:

class update_delete {
public:
    update_delete() : sum(0) {}
    bool operator()(const Child & child) {
        std::pair<int, bool> result = child.update();
        sum += result.first;
        return result.second;
    }
private:
    int sum;
}

If you cannot make update it const, just swap the element with some element from the back (you would have to keep an iterator that always points to the last element that is available for swapping). When you aggregation is done, just discard the end of the vector (that now contains all the elements that are to be deleted) using vector::resize. This is analogous to using std::remove_if, but I am not sure if it is possible/valid to use it with a predicate that modifies the objects in the sequence.