Sign in to follow this  

Delete all objects in std::vector

This topic is 2543 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

For quite some time now, I have been using this method to empty the contents of a vector.


std::vector <Object*> children;
std::vector <Object*>::iterator childrenIterator;

for(int i=0; i<100; i++)
{
children.push_back(new Object()); // Fill the vector
}

for(int i=0; i<children.size(); i++)
{
delete children[i]; // Delete object it points to

childrenIterator = children.begin()+i; // Set iterator
children.erase(childrenIterator); // Delete pointer
}


Recently, I have discovered that this in fact, does not delete all of the objects/references, only some. I know that I should be using iterators in the for loop, but

1. I'm not quite sure how to delete the object it points to with an iterator (if someone could point out how, I would appreciate it)
2. This should work anyways, just not as efficiently

Thanks in advance

Share this post


Link to post
Share on other sites
You are changing the size of the vector as you are iterating across it.

Normally you would do something like:


std::vector<Object*>::iterator i=children.begin();
while(i!=children.end())
{
delete *i;
i=children.erase(i);
}


erase() returns an iterator to the next item in the sequence, or end() if it is the last item.

Alternatively:


for(std::vector<Object*>::iterator i=children.begin();i!=children.end();++i) delete *i;

children.clear();

Share this post


Link to post
Share on other sites
If you delete ALL objects of the vector you might want to not erase every single object but do a clear after you deleted them:


std::vector<Object*>::iterator it( children.begin() );
while ( it != children.end() )
{
delete *it;
++it;
}
children.clear();

Share this post


Link to post
Share on other sites
Your code actually works, except that the vector.erase() call actually decreases the size of the array every time. So, imagine if we put 8 elements A B C D E F G H into an array, then called your function

loop iterations:
i=0,size=8,array={A,B,C,D,E,F,G,H}
i=1,size=7,array={B,C,D,E,F,G,H}
i=2,size=6,array={C,D,E,F,G,H}
i=3,size=5,array={D,E,F,G,H}
i=4,size=4,array={E,F,G,H}
stop.

as you can see, because the size changes, the erase method fails. There are 3 ways to fix this problem, in order of increasing "goodness"

method 1: store the size before:

const int s=children.size();
for(int i=0; i<s; i++)
{
delete children[i]; // Delete object it points to

childrenIterator = children.begin()+i; // Set iterator
children.erase(childrenIterator); // Delete pointer
}


method 2: clear the array after iteration. This way is better because vectors are worst case O(n) on removal, which means your algorithm could be O(n^2).

for(int i=0; i<children.size(); i++)
{
delete children[i]; // Delete object it points to
}
children.clear();


lastly, although theres technically nothing wrong with iterating through the array you did, it is better to use iterators. Theres not really a big difference if the data structure has O(1) lookup on the [] operator (like a vector does), but other data structures (like lists or deques) don't have fast random access. Therefore, doing it with iterators is MUCH better.


for(std::vector<Object*>::iterator ci=children.begin();ci!=children.end();++ci)
{
delete *ci; //delete the object the iterator points to
}
children.clear();

Share this post


Link to post
Share on other sites
Hmm, the for loop is pretty messy there (not really, but I don't feel like analyzing it XD). The error probably has something to do with you setting the iterator 1 position past the beginning (in which case, when you reach the end I think you'll be reaching into the depths of hell that will summon the devil to come for your soul - in other words, you're about to touch a fragment of memory that doesn't belong to the vector and something is going to crash)

To fix the mess and not be bothered with specific counting of the iterator positions, you can do it this way

while(!children.empty())

{

delete children.back(); // Delete object it points to

children.erase(children.end()); // Delete pointer
}


That should do what you intent without being as messy :) Good luck!

EDIT: O.O!!!!! I got TRIPLE Ninja'd O.e? FAILURE

Share this post


Link to post
Share on other sites
Thanks everyone

I am now using this approach


for(std::vector<Object*>::iterator ci=children.begin();ci!=children.end();++ci)
{
delete *ci; //delete the object the iterator points to
}

children.clear();


And it works perfectly, although I have one more question.

Should I be declaring the iterator in the for loop(above) or when I declare the vector?


std::vector <Object*> children;
std::vector <Object*>::iterator ci;

Share this post


Link to post
Share on other sites
Quote:
Original post by Dman1a
Should I be declaring the iterator in the for loop(above) or when I declare the vector?


std::vector <Object*> children;
std::vector <Object*>::iterator ci;
It's the same rule as with other variables - declare them as close as possible to where they're actually used. In the case of an iterator that often means in the for loop.

Share this post


Link to post
Share on other sites

This topic is 2543 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.

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