I seem to be having a spot of trouble dealing with erasing elements from an STL vector. The vector erase function requires a vector instead of just an index to delete. In my class I have a member variable as follows...

vector<Character*> units;

The vector gets populated as follows...

for(int i=0; i<NUM_UNITS; i++)
{
    Character* tmpchar = new Character;
    tmpchar->Load("PlayerModel.bmf", "ZombieTexture.tga", ZOMBIE_UNIT);
    units.push_back(tmpchar);
}

Here is where my error is coming from...

//Delete any dead units
for (vector<Character*>::iterator it = units.begin(); it!=units.end(); it++)
{
    if(it->GetHealth() <= 0)
    {
        //Unload the unit
        delete &it
        units.erase(it);
    }
}

The compiler shouts at me in the if statement. I have no idea how to interpret this error message. error: request for member `GetHealth' in `*(&it)->__gnu_cxx::__normal_iterator<_Iterator, _Container>::operator-> [with _Iterator = Character**, _Container = std::vector<Character*, std::allocator<Character*> >]()', which is of non-class type `Character*'| My thinking in the if statement is that the iterator is of type Character* in which case why can I not call the Character member function GetHealth? The GetHealth function is indeed there but I think the message isin't complaining about the function so much as it doesn't think its a Character pointer. Any Help? Thanks in advance!

You use the * operator to access the element pointed too by the iterator.

Try changing

delete &it

to

delete *it;

and

if(it->GetHealth() <= 0)

to

if((*it)->GetHealth() <= 0)

[edit]

A recommendation, you might think about replacing your storage of Character pointers with smart_ptr's instead. It would eliminate the need to have the delete line, all you would have to do is erase the element from the vector. Once it has been removed, and there are no other references to it, it would automatically be deleted.

Quote:Original post by Rattrap
//Omitted

A recommendation, you might think about replacing your storage of Character pointers with smart_ptr's instead. It would eliminate the need to have the delete line, all you would have to do is erase the element from the vector. Once it has been removed, and there are no other references to it, it would automatically be deleted.

Thanks for the quick reply. I will have to look into smart pointers. I have heard of them but never knew what purpose they had.

Quote:Original post by RDragon1
This is NOT how you erase elements from a vector while you're iterating over it:

//Omitted

If you're often removing from the middle, vector might be the wrong container choice

Thanks for the correction. I was aware from one of my books that deleting mid vector was not very efficient. Right now however I am unsure of the gameplay and it might not be an issue (Only 1 or 2 zombies being killed every few seconds). However, if it should present an issue such as a case where the player is spraying shotgun into a crowd and multiple units are killed every game update, what would be a suggestion for a better structure to hold the references? I could do a fixed length array and zero out the memory for a slot if it is deleted, but are there any better containers for this type of situation?

Thanks!

I'd go so far as to suggest:

it is faster, so long as the order of units in the array doesn't matter.
Since it swaps all the dead ones to the end of the array, so erase can just lop off the tail end.