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.

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.

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));


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.

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.

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.

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.

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.

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.