Suppose there's a vector of Item
s
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
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.