• Advertisement
Sign in to follow this  

Deleting from vector with iterator (99% sure this is the way to go)

This topic is 2099 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hello!

I am running this code
for(vector<Enemy*>::iterator it = enemyVector.begin(); it != enemyVector.end();)
{
if((*it)->getState())
{
delete *it;
it = enemyVector.erase(it);
}
else
{
++it;
}
}
to delete enemies which are flagged as "dead".

From what I've gathered when googleing for a couple of days this is the right way to delete with iterators from a vector. BUT when i kill the last item in the vector my program crashes.

I've had this problem for a while (although it went away for a while, but is back now so i'm guessing the problem lies elsewhere in the code).

If this is the right code i am using what other kinds of code can make it crash? Nowhere else in the program do i delete from the vecor the only other interaction with the vector is from code similar to this:
for(vector<Enemy*>::iterator it = enemyVector.begin(); it != enemyVector.end(); it++)
{
(*it)->draw();
}


Basically what i need help with is knowing what to look for since uploading my code would be to much to ask for and i don't expect anyone to look through it all since it's pretty much code.

Share this post


Link to post
Share on other sites
Advertisement
It does look in order. Are you sure that the last element in the vector is a valid pointer in the first place? It could be the pointer that is incorrect and not the way you're removing it. But I would consider using the standard library algorithms to handle correct removal of objects:
[source]
auto predicate = [](Enemy const *e)->bool {
if(e->getState()) {
delete e;
return true;
} else {
return false;
}
};

v.erase(std::remove_if(v.begin(), v.end(), predicate), v.end());
[/source]
Now you have separated the logic of determining what is being removed (the predicate) and the way elements are removed from the vector (using working standard library algorithms). The predicate, which is your own code, has nothing to do with removing elements. Likewise, the standard algorithms knows how to remove elements correctly but does not need to know how to determine what elements to remove.

If you're not into C++11 then you can provide a regular function or a function object instead of a lambda object for the predicate instead.

Furthermore, I would suggest pointer-containers instead of value-containers: boost::ptr_vector<Enemy> or std::vector<scoped_ptr<Enemy> will automatically release the pointer so you don't have to do it yourself.

Share this post


Link to post
Share on other sites
Thanks for the reply.

This is how i create my enemies atm

/***In .h File***/
#include "enemy.h"
vector<Enemy*> enemyVector;

/***In .cpp File***/
//create a enemy (int hpnumb, int x, int y, int w, int h, int sheetnumb, int tilenumb)
enemyVector.push_back(new Enemy(40, 384, 64, 64, 64, 1, 0));
enemyVector.push_back(new Enemy(40, 384, 128, 64, 64, 1, 1));
Edited by Tallkotten

Share this post


Link to post
Share on other sites
But what does the vector contain when you try to remove the last element? That's where it's crashing. Use the debugger to inspect the vector when it crashes and see what it contains and why it crashes on the last one and not any of the other elements in the vector.

Share this post


Link to post
Share on other sites
By changing some code in my imageloader class it's not crashing anymore... Weird, i'm guessing i screwed up somewhere, will probably pick it up sometime x)


But what does the vector contain when you try to remove the last element? That's where it's crashing. Use the debugger to inspect the vector when it crashes and see what it contains and why it crashes on the last one and not any of the other elements in the vector.


I've actually never debugged in codeblocks and i have no idea how it works. Do you know any tutorials i can learn from? Since if i knew how to debug properly a lot of these small mistakes could be easily solved.

Share this post


Link to post
Share on other sites
I just searched for codeblocks and debugging and got quite a few results. Looks like it works pretty much like the Visual Studio debugger, and I would assume like most other visual debuggers as well.

Share this post


Link to post
Share on other sites
This is not the canonical method of erasing elements from a vector. Take a look at remove_if in algorithms. The typical problem is that the iterator is invalid after the erase.

iterator it = remove_if(enemyVector.begin(), enemyVector.end(), YourPredicate);

Then use this new iterator to visit the removed elements to delete them. I would then revisit to erase all in one block.

for_each(it, enemyVector.end(), DeleteFunction);
enemyVector.erase((it, enemyVector.end());

Three functions calls, two simple (inline) functions.

Share this post


Link to post
Share on other sites
Bill Door, that is a similar but more elongated version of what Brother Bob was talking about.
Why make a special delete function and use a for_each? that seems a waste of time, since you can just do the delete in the predicate function.

this is what i did on a homework assignment where i had to delete dead game actors

del_func (takes object type or type pointer) { if (dead) delete}

iterator newEnd = remove_if (vec.begin(), vec.end(), del_func);

vec.erase(newEnd, vec.end());



again this is all similar to what Brother Bob already mentioned

Share this post


Link to post
Share on other sites

I just searched for codeblocks and debugging and got quite a few results. Looks like it works pretty much like the Visual Studio debugger, and I would assume like most other visual debuggers as well.


I've actually never debugged before and when i read a tutorial earlier today i didn't not get all of the words used. Guess i'll have to find one that expects no previous knowledge of the subject.

Share this post


Link to post
Share on other sites
Above solutions should all work.. but if order doesn't matter and extra speed is warranted:

// If order doesn't matter, swap to back and and pop. This wont move the
// elements after current iterator during erase, but the order of elements
// will change.
for (vector<Enemy*>::iterator it = enemyVector.begin(); it != enemyVector.end(); )
{
if ((*it)->getState())
{
delete *it;
// Swap to back and pop, and delete it from there, but don't swap the last element.
if (enemyVector.size() > 1)
std::swap(*it, enemyVector.back());
enemyVector.pop_back();
}
else
{
++it;
}
}



Edit: Disregard that, I'll edit another solution (different from above)
Edit2: there. Edited by Codarki

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement