Search code examples
c++vectorself-destruction

What will happen if a std::vector element 'commits suicide' (using delete this;)?


Suppose there's a vector of Items

vector<Item*> items; //{item1, item2, item3}

Then, in other part of the code,

items[1]->suicide();

where the suicide function is:

void Item::suicide()
{
   delete this;
}

What is items vector size and how it's arrangement now? It is okay to do this?

Edit (may I ask an additional question?): If the desired arrangement of the output is {item1, item3}, size is 2, and no dangling pointer, how to do it in a self-destructing way (from the item2 itself)?

Edit 2 : Thanks for all the answers! Awesome. So I finally decided and found the way to do it from outside of the object because it was a bad practice and unnecessarily complicated


Solution

  • What is items vector size and how it's arrangement now? The same. The function call does not change the vector contents nor size at all. It just frees the memory the pointer is pointing to.

    Is it okay to do this? More precisely: Is it legal C++? Yes. Is it good style programming? No. Let me elaborate on the latter:

    • There should be a separation of concerns: Who's responsible for memory management? The container or the user of the class Item or the class Item itself?

    • Typically the container or user should do that, because he knows what's going on.

    • What's the way to do that? Memory management in modern and safe C++ code is mostly done using smart pointers like std::shared_ptr and std::unique_ptr and containers like std::vector and std::map.

    • If the class Item is a value type (that means you can simply copy it and it has no polymorphic behavior in terms of virtual functions), then just use std::vector<Item> instead for your code. Destructors will be called automatically as soon as an element is removed from the container. The container does it for you.

    • If the class Item has polymorphic behavior and can be used as a base class, then use std::vector<std::unique_ptr<Item>> or std::vector<std::shrared_ptr<Item>> instead and prefer the std::unique_ptr solution, because it adds less overhead. As soon as you stop referring to an object, it will be deleted automatically by the destructor of the smart pointer you are using. You (almost) don't need to worry about memory leaks anymore.

    • The only way you can produce memory leaks is that you have objects that contain std::shared_ptrs that refer to each other in cyclic way. Using std::unique_ptrs can prevent this kind of trouble. Another way out are std::weak_ptrs.

    Bottom line: Don't provide a function suicide(). Instead put the responsibility solely into the hands of the calling code. Use standard library containers and smart pointers to manage your memory.

    Edit: Concerning the question in your edit. Just write

    items.erase( items.begin() + 1 );
    

    This will work for all types: std::vector of values or pointers. You can find a good documentation of std::vector and the C++ Standard library here.