If I have class and vector of pointers in class how should I write destructor to properly free memory? Is it correct?
It depends.
Do the pointers point to objects that were allocated with new
and is this class the owner of those dynamic objects (i.e. is this class responsible for their destruction)? If not, then it's not correct.
Or should I also erase elements from vector?
Your example for erasing elements of the vector is wrong. It will skip half of the elements and thus will not call delete all pointers. After first iteration, first element will have been erased, so the next element has been shifted to the index 0, but next loop deletes and erases the element in index 1.
There's no point in erasing elements of the vector, because it is immediately about to be destroyed.
Or any other solutions?
If you intend for the class to own dynamically allocated objects, prefer using a vector of objects instead of pointers. If you need a vector of pointers for example to allow run time polymorphism, then use a vector of std::unique_ptr
. That way you don't need to implement a destructor as the automatically generated would be just fine.