Vector delete problem?

[Calender Man]

Hey, when I delete the last object in my vector I get the debug assertion failed error saying the vector subscript is out of range. I'm using
object.erase(object.begin());

to delete the objects and it works as long as there are other objects on the screen, but when it comes down to the last one it just fails. I think it means its trying to access an object thats no longer there but even using

if(!object.empty())

it still gives the error. Am I deleting the objects incorrectly?

[Gaminic]

First of all, if you're calling erase(.begin()) that often, you're probably using the wrong container class (I think deques are specialized in front/end access). A vector is mainly useful when most of the action takes place on the last item. Generally, that means static content of unknown size (i.e. adding an unknown amount of items, then leaving the size constant).

Secondly, it's difficult to pinpoint the problem without any context. At first glance, there shouldn't be anything wrong with the code you provided. Can we see the surrounding code?

[Calender Man]

I used a vector as the objects are bullets created when the space key is pressed. Heres the surrounding code:

if(keys[VK_SPACE]) 
{
       bullet.push_back(new Bullet);
       fired=true;
}

if(fired == true)
{
for (int i = 0; i != bullet.size(); i++)
{
	drawSprite(*bullet[i]);
					
	if(bullet[i]->getX() > 400)
	{
		bullet.erase(bullet.begin());
		if(bullets.empty())
		{
			fired = false;
		}
	}
}

[coder777]

You're checking for empty() just too late. After erase is already done.

you shouldn't use a for loop with index when erase is involved. Use iterators instead

[Gaminic]

Here's a hint: NEVER use for loops for iterating over a vector if you plan on deleting elements. Sure, you can do it just as easily, but "blind-index-incrementing" is dangerous if you don't know how vectors work. A while loop and manual incrementing gives you far more control and forces you to think on its behavior.

An example: the vector of ints V{8, 4, 8, 8, 9}, and you wish to delete all 8's

i = 0: V[0] = 8 -> erase -> {4, 8, 8, 9}, ++i.
i = 1: V[1] = 8 -> erase -> {4, 8, 9}, ++i.
i = 2: V[2] = 9 -> skip, ++i.
i = 3 = V.size() -> end.

If you delete element X, all elements ]X, size[ are copied to [X, size-1[. Due to your auto-incrementing, you skipped several elements. Not a disaster, but unexpected behavior nontheless.

Now, try the example above again with the vector {4, 4, 4, 8} and see if you can figure out why your problem crashes. Hint: the problem is your for's stopping condition.

[johnbob]

Here is your problem ! :

by deleting the first element of your vector , you are shifting everything left (cf http://www.cplusplus.com/reference/stl/vector/erase/ :

Because vectors keep an array format, erasing on positions other than the vector end also moves all the elements after the segment erased to their new positions, which may not be a method as efficient as erasing in other kinds of sequence containers (deque, list).

And you are doing this in a loop iterating over the indices of that same vector

[Calender Man]

I'm not sure I follow, if it was {4, 4, 4, 8} it would just be:

i = 0: V[0] = 8 -> erase -> {4, 4, 4, 8}, ++i.
i = 1: V[1] = 4 -> skip, ++i.
i = 2: V[2] = 4 -> skip, ++i.
i = 3 = V.size() -> end.

Not sure how that would make it crash.

[ne555]

... ok, try {8, 4, 4, 4} then.

	if(bullet[i]->getX() > 400)
	{
		bullet.erase(bullet.begin());

¿So the 42th goes out of town, and you punish the 1st?

bullet.push_back(new Bullet); Bad use of pointers there, possibly a memory leak.

[Calender Man]

So I go this problem fixed by using break instead of fired = false. I'm having another problem though, after I shoot around 50+ bullets my game deletes all the sprites on the game. I think this means its run out of memory so is there something more I should be doing after erase?

[andywestken]

If you new something, it's your responsibility to delete it.

Your vector only stores the pointers to bullet objects. erase() removes the pointer but does nothing about freeing the memory.

if(keys[VK_SPACE]) 
{
       bullet.push_back(new Bullet);
       fired=true;
}

if(fired == true)
{
    for (vector<Bullet*>::iterator iter = bullet.begin(); iter != bullet.end(); /* no ++*/)
    {
        Bullet* pbullet = *iter;

        drawSprite(pbullet);
					
        if(pbullet->getX() > 400)
        {
            iter = bullet.erase(iter); // erase updates iterator
            delete pbullet; // free the bullet that's no longer needed
            if(bullet.empty())
            {
                fired = false;
            }
        }
        else
        {
            ++iter; // inc iterator as erase() not called
        }
    }
}

Andy

P.S. Do you need fired? Or could you just test whether or not bullets.empty() is true?

[Calender Man]

Thanks, appreciate the help. I tried it out the way you posted but it still happens, really confused :S.

And yeah I never thought of just checking for empty, will do that instead.

[andywestken]

"it still happens" refers to your shooting 50+ bullets problem?

[Calender Man]

Yeah, after too many bullets the sprites delete themselves and the game freezes.

Edit: I think I've fixed the problem, I was loading the sprite in every frame instead of just once when it was created. Thanks for your help :)