1

Ok, this is probably pretty basic, but I've never done it before.

I have a vector of Particles, and when they leave the visible screen, I want to erase them. I looked up the erase-remove idiom, but I don't know how to make that work, because I also need to delete the Particle instances. I tried with backwards iteration, without success:

for ( std::vector<Particle*>::reverse_iterator rit = particles.rbegin(); rit != particles.rend(); ++rit )
{
    if ( IsOffScreen((*rit)->pos) )
    {
        delete (*rit.base());
        particles.erase(rit.base());
    }
}

At runtime crash, Visual Studio says "iterator cannot be decremented". What am I doing wrong? Is there a better way?

Innkeeper
  • 663
  • 1
  • 10
  • 22
  • 1
    You mustn't say `++rit` when `rit` isn't a valid iterator. – Kerrek SB Oct 20 '13 at 14:26
  • 1
    You will find the return value of [`std::vector::erase`](http://en.cppreference.com/w/cpp/container/vector/erase) particularly helpful in solving your issue, as it should be become the new iterator value rather than incrementing/decrementing the exiting one in the event of an erasure. – WhozCraig Oct 20 '13 at 14:29
  • Make this with indexes in reverse order. – Alex F Oct 20 '13 at 14:30
  • If you used smart pointers, you could use the erase-remove idiom and stop worrying about leaks. – syam Oct 20 '13 at 14:31
  • @syam Yes I might do that, but I wanted to measure performance with a lot of particles before, and compare smart pointers vs raw pointers. – Innkeeper Oct 20 '13 at 14:31
  • @Innkeeper `unique_ptr` has **zero** overhead compared to raw pointers, it's just template magic. `shared_ptr` is another story, of course, but since your vector obviously owns the particles, `unique_ptr` would be appropriate here. – syam Oct 20 '13 at 14:33
  • possible duplicate of [How to erase & delete pointers to objects stored in a vector?](http://stackoverflow.com/questions/991335/how-to-erase-delete-pointers-to-objects-stored-in-a-vector) – syam Oct 20 '13 at 14:37
  • @syam Thank you, I did not know that. I'll probably use that then! – Innkeeper Oct 20 '13 at 14:52

1 Answers1

1

What about:

for (std::vector<Particle*>::iterator it = particles.begin(); it != particles.end(); ++it) {
    if (IsOffScreen((*it)->pos)) {
        delete *it;
        *it = NULL;
    }
}
particles.erase(std::remove(particles.begin(), particles.end(), NULL), particles.end());

There are various other options - you could actually delete them in the predicate, for example, since IIRC it's guaranteed to be run exactly once on each item.

You do need to be careful with remove and remove_if for this kind of thing, since the removed objects are left in undefined state, so it's not possible to remove them and delete afterwards (you can use std::partition for that, but it's slower).

Peter
  • 7,216
  • 2
  • 34
  • 46