Sign in to follow this  
3DModelerMan

C++ deletion from vector

Recommended Posts

I don't understand where this code doesn't work, I know that isDead() returns true when I hit the enemy but the destructor for the object never gets called. for ( int i=0; i<mEntityList.size(); ++i ) { mEntityList[i]->update(); if ( mEntityList[i]->isDead() ) { mEntityList.erase(mEntityList.begin()+i);//Delete dead people. //delete mEntityList[i]; } } I commented out my other way of deleting cause it didn't work either.

Share this post


Link to post
Share on other sites
You are erasing the element from the vector, then deleting a (unrelated) entity.

Instead, either delete first, or save the pointer and delete afterwards:

for ( int i = 0 ; i < mEntityList.size() ; )
{
Entity *entity = mEntityList[i];

entity->update();

if ( entity->isDead() )
{
mEntityList.erase(mEntityList.begin()+i);//Delete dead people.
delete entity;
}
else
{
++i;
}
}



Note you can use [source][/source] or [code][/code] to preserve the formatting of posted code.

[edit:

I forgot to make the increment conditional. I usually use std::remove_if() which is a better algorithm to use:

#include <algorithm>

struct UpdateAndDeleteDeadEntities
{
bool operator()(Entity *entity)
{
entity->update();

bool dead = entity->isDead();
if(dead)
{
delete entity;
}
return dead;
}
};

// whereever your code was
mEntityList.erase(
std::remove_if(mEntityList.begin(), mEntityList.end(), UpdateAndDeleteDeadEntities()),
mEntityList.end() );

Share this post


Link to post
Share on other sites
Two problems I see:
1. Assuming the elements in the vector are objects created with new, you need to delete them.
2. You're erasing elements in the vector as you iterate through it - you'll miss elements in the vector.

This should work:

for (std::vector<Entity*>::iterator it=mEntityList.begin(); it!=mEntityList.end();)
{
Entity* pEntity = *it;
pEntity->update();
if(pEntity->isDead())
{
it = mEntityList.erase(it); // Remove from the vector
delete pEntity; // Delete the entity and free memory
}
else
++it;
}

Share this post


Link to post
Share on other sites

std::for_each(mEntityList.begin(), mEntityList.end(), std::mem_fun(&Entity::update));
mEntityList.erase(std::remove_if(mEntityList.begin(), mEntityList.end(), std::mem_fun(&Entity::isDead)), mEntityList.end());


That doesn't do any deletion, though. Consider using smart pointers instead. You'll need to make wrapper functions to call the update and isDead functions (since std::mem_fun is expecting a raw pointer), but the basic logic is the same.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this