2

If I have class and vector of pointers in class how should I write destructor to properly free memory? Is it correct?

~Class()
{
  for(int i = 0; i<v.size(); i++)
  {
    delete v[i];
  }
}

Or should I also erase elements from vector? Or any other solutions?

~Class()
{
  for(int i = 0; i<v.size(); i++)
  {
    delete v[i];
    v.erase(e.begin() + i);
  }
}
Ayaka
  • 59
  • 6
  • 5
    Use a vector of `std::unique_ptr` it will do the job for you. – Richard Critten Jan 10 '18 at 20:43
  • Possible duplicate of [Correct allocate and free memory for arrays in C++](https://stackoverflow.com/questions/4298314/correct-allocate-and-free-memory-for-arrays-in-c) – Stefan Falk Jan 10 '18 at 20:44
  • Do you have to erase the elements of a `vector`? – juanchopanza Jan 10 '18 at 20:44
  • If you are dealing with compiler without C++11 support then you can check [`::boost::ptr_vector`](http://www.boost.org/doc/libs/1_66_0/libs/ptr_container/doc/ptr_vector.html) – user7860670 Jan 10 '18 at 20:45
  • i think that the first approach will be better because you do not need to erase the elements in the vector, the destructor of the vector will do it for you – Kfirka Jan 10 '18 at 20:49
  • I know that I could use unique_ptr but I'd like to ask about new and delete. No I don't have ot erase the elements of vector but I have read that I should use erase after deleting – Ayaka Jan 10 '18 at 20:50

2 Answers2

4

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.

eerorika
  • 232,697
  • 12
  • 197
  • 326
0

If your class is to supposed to own the objects pointed to, then its sufficient to do:

for (auto p : v) delete p;

If the vector itself is the member of the class then its elements are getting deleted when the class instance object is destroyed, you dont need to do this manually. And in your example you are also doing it wrong. If its a std::vector you could just call vector's clear() method. Otherwise you would call delete[] p; where p is the pointer to the vector elements itself.

StPiere
  • 4,113
  • 15
  • 24