• Advertisement

• Popular Now

• 13
• 27
• 9
• 9
• 20
• Advertisement
• Advertisement
• Advertisement

Safely deleting a class instance

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

Hi Guys,

I have a vector of classes. How do I safely delete an item in the vector so the destructor gets called.

At the moment I have the following. But I suspect this is a 'brute force' removal.

How do I call 'delete' properly?

for(it=VectorAsset.begin();it<VectorAsset.end();it++) { if(szNameOrType==it->szName||szNameOrType==it->szType) { //delete it; // <<< wont compile VectorAsset.erase(it); } }

Any help would be awesome

Share this post

Share on other sites
Advertisement
How did you allocate those class instances ?

If you used new to allocate them you should be able to delete them using

delete *it; // it is a interator pointing to your pointer so you have to dereference it to get your pointer.
then remove the iterator using it = VectorAsset.erase(it); //you have to reassign the iterator since erase will invalidate it.

If you didn't allocate your instances with new you can just call erase and things will get cleaned up, (You still have to reassign the iterator though.

Share this post

Share on other sites
Thanks for the reply.

I allocated the instances like this;

Asset *cAsset=new Asset(szName,szTexture); VectorAsset.push_back(*cAsset);

But, Still can't call delete (where commented above);

Compiler give this error

error C2440: 'delete' : cannot convert from 'Asset' to 'void *'

This is what I have now;

void SpriteUnload(string szNameOrType) { // Erase any valid items from vector array std::vector<Asset>::iterator it; for(it=VectorAsset.begin();it<VectorAsset.end();) { if(szNameOrType==it->szName||szNameOrType==it->szType||szNameOrType=="ALL") { delete *it; //VectorAsset.erase(it); VectorAsset.erase(it); } else it++; } }

delete &it; worked, though. Edited by lonewolff

Share this post

Share on other sites
really? It worked?
That seems to be a coincidence.

 Asset *cAsset=new Asset(szName,szTexture); VectorAsset.push_back(*cAsset); 

These 2 lines actually push a copy of the asset in the vector. The original Asset pointer is lost in the process.
If you want to store asset pointers use a vector<Asset*> not vector<Asset> and directly push the pointer (without dereferencing).

Since you're posting in "for beginners" I would recommend you read up on smart pointers, especially auto_ptr which is useful in this case. Edited by Madhed

Share this post

Share on other sites

delete &it; worked, though.

Thats because you were dereferencing your pointer before pushing it to the vector, which would AFAIK create a copy of it (giving you a memory leak).

If you are going to store raw pointers you should use std::vector<Asset *> , if you use std::vector<Asset> you shouldn't allocate them using new.

Edit: Beaten by madhet,

Anyway, if you use std::vector<Asset *> instead it will work with delete *it; and you won't get a memory leak.

You could also look into using smart pointers to avoid the hassle. Edited by SimonForsman

Share this post

Share on other sites

delete &it; worked, though.
no no no no! You're trying to [font=courier new,courier,monospace]delete[/font] memory that you didn't allocate with [font=courier new,courier,monospace]new[/font]! That memory belongs to the vector/iterator, not to you.

Asset *cAsset=new Asset(szName,szTexture); VectorAsset.push_back(*cAsset);//makes a new Asset inside the vector, copies your new asset's value the vector's new asset's value //your original new asset is leaked (cAsset is not put into the vector)[/quote]With a vector of values, this should just be:Asset cAsset(szName,szTexture);//make a new temporary asset on the stack (new not required). This temporary one will be destructed when it goes out of scope. VectorAsset.push_back(cAsset);//create a new asset in the vector, copy yours into that new one //N.B. new/delete aren't required. Destructor will be called upon erasing items from the vector.
OR
you should use a vector of pointers, in which case new/delete would work as you expect them to:Asset *cAsset=new Asset(szName,szTexture); VectorAsset.push_back(cAsset);//put the pointer into the vector ... it=VectorAsset.begin() ... delete *it;//retrieves your original pointer, deletes the allocation that it points to Edited by Hodgman

Share this post

Share on other sites
Thanks for the advice guys!

I have change the code to the following

std::vector<class Asset>(VectorAsset);

So, when I call VectorAsset.erase(it); does the destructor get called still? As I have alot of DirectX code that needs to be cleaned up via the destructor (in each instance).

Share this post

Share on other sites

Thanks for the advice guys!

I have change the code to the following

std::vector<class Asset>(VectorAsset);

So, when I call VectorAsset.erase(it); does the destructor get called still? As I have alot of DirectX code that needs to be cleaned up via the destructor (in each instance).

Yes, the destructor will get called.

Share this post

Share on other sites
A wierd one. Testing my destructor, it is actually getting called when the class is created. Any ideas why that would be?

~Asset() { std::cout<<"Destructing: "<<szName<<"\r\n"; }

Share this post

Share on other sites
When you resize a vector, some implementations create a dummy instance, then use that dummy instance in the copy constructor of every new instance in the vector and then free the dummy instance. I guess it is that behavior, that you are seeing.

Share this post

Share on other sites

• Advertisement