for(vector<Projectile*>::iterator Iter = ProjectileList.begin(); Iter != ProjectileList.end(); ++Iter)
{
(*Iter)->Move(deltaT);
if (!(*Iter)->inBounds())
{
Projectile* temp = (*Iter);
ProjectileList.erase(Iter);
delete temp;
}
}Why does erasing from this vector crash my program?
Author
I have pinpointed a crash to this bit of code.
The canonical loop for erasing while iterating over a vector looks like:
for (iterator i = vec.begin(); i != vec.end()) { if (condition(i)) { i = vec.erase(i); } else { ++i; }}
Author
Thanks, that did it. Not sure why. I guess the elements are all shifted when I delete, so I shouldn't increment the iterator. If I do, then I'll increment into some undefined slot or something.
Anyway, code works now. Thanks.
Anyway, code works now. Thanks.
erasing from a vector invalidates all its iterators. Therefore after "ProjectileList.erase(Iter);" Iter is no longer valid.
That's why you need to reacquire a new one as shown in "no such user" sample.
That's why you need to reacquire a new one as shown in "no such user" sample.
template <class T, class Pred> void EraseRemoveIf(vector<T>& vec, Pred pred){ vec.erase(remove_if(vec.begin(), vec.end(), pred), vec.end());}[Edited by - Ariste on April 26, 2010 8:30:23 PM]
Quote:Original post by voodoohaust
erasing from a vector invalidates all its iterators.
Nope, invalidates all iterators AFTER the point of erasure. The problem is then that itor (which is still valid, as it points to where the erased element WAS, and hence is not AFTER the point of erasure) might equal end() and hence will be incremented by the for loop (which is illegal). A minor semantics issue, but a relatively important one to be sure.
Erasing elements one-by-one from a vector is O(n*n). I suggest the following O(n) alternative:
#include <algorithm>#include <functional>std::for_each( ProjectileList.begin(), ProjectileList.end(), std::bind2nd( std::mem_fun(&Projectile::move), deltaT ));ProjectileList.erase( std::remove_if( ProjectileList.begin(), ProjectileList.end(), std::not1( std::mem_fun(&Projectile::inBounds) ) ), ProjectileList.end());
Whoops, didn't see the delete...
It's starting to become a little silly even to my trained STL eyes, though. Just use a vector of smart pointers with my first solution ;)
[Edited by - DevFred on April 27, 2010 2:00:01 PM]
template<class T>struct delete_functor{ void operator()(T* p) { delete p; }};std::for_each( ProjectileList.begin(), ProjectileList.end(), std::bind2nd( std::mem_fun(&Projectile::move), deltaT ));std::vector<Projectile*>::iterator it = std::partition( ProjectileList.begin(), ProjectileList.end(), std::mem_fun(&Projectile::inBounds));std::for_each( it, ProjectileList.end(), delete_functor<Projectile>());ProjectileList.erase(it, ProjectileList.end());It's starting to become a little silly even to my trained STL eyes, though. Just use a vector of smart pointers with my first solution ;)
[Edited by - DevFred on April 27, 2010 2:00:01 PM]
or just wrap it all up in a single predicate...
projectiles.erase(std::remove_if(projectiles.begin(), projectiles.end(), [delta](Projectile* p) -> bool { p->move(delta); if(!p->inBounds()) { delete p; return true; } return false; }), projectiles.end() );This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement