Using "Vector.erase()" broke my program during runtime, Why did this fix it?

[keanedawg]

Prior to a small change, I couldn't get my program to work right. I'm making Asteroids and I wanted to delete ( or "erase") bullets when they leave the screen. This is an incomplete function and more code is still being added, but the problem I was originally having was fixed by changing how the incrementor works in the for loop. I don't see what I'm doing differently.

To be clear, my program only broke once a bullet left the screen, meaning the if statement returned true. I can't remember what the clearly stated.

I'm not sure why this fixes it. This is my main question.

// THIS IS WHAT I WAS ORIGINALLY DID WHICH BROKE IT DURING RUNTIME


void Asteroids::advance()	{

	// put collision checks before move commands;


	this->ship.move();

	int i = 0;

	for (vector<Bullet>::iterator it = bullets.begin(); it != bullets.end(); ++it){
		it->move();
		if (hasLeftScreen(it->getPoint())){
			it = bullets.erase(it);
		}
	}

}


// THIS APPEARS TO FIX EVERYTHING

 void Asteroids::advance()	{

	// put collision checks here

	this->ship.move();

	int i = 0;

	for (vector<Bullet>::iterator it = bullets.begin(); it != bullets.end(); /*nothing here for now at least */){
		it->move();
		if (hasLeftScreen(it->getPoint())){
			it = bullets.erase(it);
		}
		else{
			++it;
		}
	}

}



// My container for bullets

class Asteroids {


Private:
    vector<Bullet> bullets;


}

[Disch]

	for (vector<Bullet>::iterator it = bullets.begin(); it != bullets.end(); ++it){
		it->move();
		if (hasLeftScreen(it->getPoint())){
			it = bullets.erase(it);
		}
	}

erase() will return the iterator to the element after the one you just erased.

So if you have a vector that contains {0,1,2,3}, and you erase() the 2, your new vector will be {0,1,3} with the iterator pointing to the 3 element.


Your problem with the above for loop is that erase kind of increments your iterator for you -- whereas in your for loop you are manually incrementing. So take a look what happens when you erase the last element in the vector:

// bullets =  {0,1,2,3}
//
// Bullet 3 has left the screen, so you erase it.
it = bullets.erase(it);

// now:
// bullets = {0,1,2}
// it = bullets.end()  (the element after the erased '3')

// then, when your loop finishes
++it;

// now:
// it = bullets.end()+1  -- OUT OF BOUNDS 

Now your loop will keep going because it != end(), and any time you try to dereference it, you go out of bounds, possibly causing a crash.



Your second loop fixes this by only incrementing when you don't erase.



For this reason, loops that iterate over elements while erasing some of them are typically done with while loops, and not with for loops:

auto it = bullets.begin();
while( it != bullets.end() )
{
    if( ... )
        it = bullets.erase(it);
    else
        ++it;
}

[keanedawg]

Thank you so much