Erasing vector element that is off screen

[slouchcouchgamer]

I am playing around with SFML, and have a small test set up where I can click on the screen and a "bomb" drops from that point towards the bottom of the screen. Each click calls a function to emplace_back into a vector of class Bomb:

void createBomb(std::vector<Bomb>& bombs, int x, int y)
{
	bombs.emplace_back(x, y);
}

Once the bomb hits the ground, it is flagged as isDestroyed = true. I want that instance to be deleted from memory. And so I have the code below.

for (int i = 0; i < bombs.size(); ++i)
	{
		if (bombs[i].isDestroyed)
			bombs.erase(std::begin(bombs));
	}

It works, to a point...the bomb is erased, but it requires the first bomb released to be the first to hit the ground. I am happy that I eventually got it roughly working, but I know that it is horrible code, and would really appreciate if somebody could point me towards a more elegant and more correct way to do this.

[hyperfine]

Try this:

bombs.erase(bombs.begin() + i)

The bombs.begin() part may not be necessary, but it's what I have in my code for something similar I'm doing, and it works.

[cire]

Even with the modification suggested by hyperfine, your code will have issues with consecutive bombs that need to be removed.

The canonical way to do this is to use the erase-remove idiom:

#include <algorithm>

    // ...
  
    auto is_destroyed = [](const Bomb& b) { return b.isDestroyed; };
    std::erase(std::remove_if(bombs.begin(), bombs.end(), is_destroyed), bombs.end());

    // ... 

otherwise, you might try using a while loop with iterators instead of an index, and taking advantage of the return value for std::vector::erase.

[slouchcouchgamer]

Thanks cire. I'll have to analyse that snippet when I get home from work, I don't quite understand remove_if, but that looks like doing the job.

About iterating over the vector; that was my first idea. But I was constantly getting invalid iterator errors. I know that using std::erase on a vector will invalidate an iterator, so I presume that was the cause, but I couldn't figure out a way around it.

Are you saying that with the return value of std::vector::erase (which I presume is an iterator), you can re-validate the iterator? Do you assign the position of the flagged element to a temporary iterator, call std::erase, and then continue with the temporary iterator? Do you reset the iterator to the beginning of the vector?

[MiiNiPaa]

Do you assign the position of the flagged element to a temporary iterator, call std::erase, and then continue with the temporary iterator?

Close to it:

auto it = bombs.begin();
while (it != bombs.end()) {
    if (it -> is_destroyed)
        it = bombs.erase(it);
    else
        ++it;
}

[slouchcouchgamer]

Excellent! Thank you very much MiiNiPaa.

[MiiNiPaa]

However erase-remove idiom should be preferred instead of this. It is faster and easier to read.