Jump to content
  • Advertisement
Sign in to follow this  
MajinMusashi

Releasing memory from a vector of pointers

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

I have a vector of pointers, and seems to me (well, to the compiler too) that memory is not being correctly freed. Take a look at the source:
std::vector<MyItem*> itemVector;
...
std::vector<MyItem*>::iterator it;
for ( it = itemVector.begin(); it != itemVector.end(); ++it ){
    delete *it; *it = NULL; // Release the pointed memory
    itemVector.erase( it ); // Remove only the pointers, not the pointed memory
}


I have [google] for it but found nothing. Can you show me the correct way to do that? Thanks a lot! [Edited by - MajinMusashi on August 8, 2005 12:42:39 AM]

Share this post


Link to post
Share on other sites
Advertisement
That ought to work. Did you make sure the MyItem destructor is virtual? EDIT: Assuming you have derived from MyItem and are instantiating new objects of its derived classes.

Share this post


Link to post
Share on other sites
Hmmmmm...that looks okay. Well according to how I've done that in the past:


std::vector<SiSESurface*> Surfaces;
typedef std::vector<SiSESurface*>::const_iterator csurfaceIter;

if (Surfaces.size() > 0)
{
csurfaceIter suIter = Surfaces.begin();
csurfaceIter suEIter = Surfaces.end();
for (; suIter != suEIter; suIter++)
{
delete *suIter;
}
}



What errors are you getting, and in which bit of code?

Share this post


Link to post
Share on other sites
Quote:
Original post by load_bitmap_file
That ought to work. Did you make sure the MyItem destructor is virtual? EDIT: Assuming you have derived from MyItem and are instantiating new objects of its derived classes.

No, MyItem class doesn't inherit from any class, and I'm sure the MyItem::~MyItem is being called.
Thanks!

Share this post


Link to post
Share on other sites
It is a common error: erase() invalidates vector iterators.

std::vector<MyItem*>::iterator it = itemVector.begin();
while(it != itemVector.end)
{
delete *it;
it = itemVector.erase(it);
}


Or, simpler
std::vector<MyItem*>::iterator it;
for ( it = itemVector.begin(); it != itemVector.end(); ++it )
delete *it;
itemVector.clear();

Share this post


Link to post
Share on other sites
Quote:
Original post by garyfletcher
What errors are you getting, and in which bit of code?


I'll try to do a better explanation:

// MyItem doesn't inherit from any class, and MyItem::~MyItem() is being called
std::vector<MyItem*> itemVector;
...
std::vector<MyItem*>::iterator it;
for ( it = itemVector.begin(); it != itemVector.end(); ++it ){
delete *it; *it = NULL; // Release the pointed memory
itemVector.erase( it ); // Remove only the pointers, not the pointed memory
}


When there is only one element inside the vector, the first iteration goes well, but then the execution point iterates AGAIN into the for loop, delete something (I don't know what, because the vector is empty!!) and freezes the application when "itemVector.erase( it )" is called.

Thanks a lot!

Share this post


Link to post
Share on other sites
Quote:
Original post by Fruny
It is a common error: erase() invalidates vector iterators.
std::vector<MyItem*>::iterator it;
for ( it = itemVector.begin(); it != itemVector.end(); ++it )
delete *it;
itemVector.clear();

Wow, thanks, Fruny! It works perfectly now!
Goodbye!

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
I have a feeling that a call to erase messes up the iteration so you should either iterate through the vector and delete all the pointers and then call
itemVector.erase(itemVector.begin(), itemVector.end())
to clear the vector or you can rewrite the loop so that it uses the call to erase to advance the iterator like this:

std::vector::iterator it = itemVector.begin();
while (it != itemVector.end())
{
delete *it; *it = NULL; // Release the pointed memory
it = itemVector.erase(it);
}


Making the vector a vector smart pointers which automatically deallocate their memory might be a better approach.

Share this post


Link to post
Share on other sites
Quote:
Original post by MajinMusashi
Quote:
Original post by load_bitmap_file
That ought to work. Did you make sure the MyItem destructor is virtual? EDIT: Assuming you have derived from MyItem and are instantiating new objects of its derived classes.

No, MyItem class doesn't inherit from any class, and I'm sure the MyItem::~MyItem is being called.
Thanks!


If polymorphism is not required, consider storing actual objects in the container rather than instances. Pointers here add additional complexity - space (the pointers), time (cache efficiency - with some containers at least - and time for performing the actual indirection), and coding (since there is now a question of "object ownership"). You should generally only be using pointers if you need it for polymorphism, or if you *want* to share the objects (and in most of these cases, the container will not "own" the objects).

That said, Fruny's advice is technically correct and I'm glad you have it working now. :)

Share this post


Link to post
Share on other sites
Quote:

If polymorphism is not required, consider storing actual objects in the container rather than instances. Pointers here add additional complexity - space (the pointers), time (cache efficiency - with some containers at least - and time for performing the actual indirection), and coding (since there is now a question of "object ownership"). You should generally only be using pointers if you need it for polymorphism, or if you *want* to share the objects (and in most of these cases, the container will not "own" the objects).

I'm using somethig like "std::vector<CElement*> elemVector" where CElement is an abstract base class used by my GUI elements. I don't know if this is the case you're talking about.

Thanks anyway!

Share this post


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

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!