CODEWITHSUNDEEP
c++pointersvectordestructor
Willem Decloedt
Willem Decloedt

Reputation: 3

c++: how to write a destructor for a vector combined with a pointer

I need to write a destructor for an exercise of school. I already tried to write the destructor for class A, is this the right or the wrong way?

Exercise on paper

The exercise says the destructor needs to start like:

A *pa = .......;
delete pa;

The code of the 4 classes:

Class A
{
  private:
    vector <B*> b;
    vector <C*> c;
  public:
    ~A();
}

Class B
{
  private:
    vector <D*> d;
  public:
    ~B();
}

Class C
{
  private:
    vector <D*> d;
  public:
    ~C();
}

Class D
{
  private:
    vector <A*> a;
  public:
    ~D();
}

I've already tried to write the destructor for class A, is this the correct way? 

~A()
{
  for (int i = 0; i < b.size(); i++)
  {
     B* pa = b[i];
     delete pa;
  }

  for (int j = 0; j < c.size(); i++)
  {
    C* pa = c[i];
    delete pa;
  }

Upvotes: 0

Views: 1265

Answers (3)

eerorika
eerorika

Reputation: 238351

It depends. If A owns the pointers in the vectors and is therefore responsible for the destruction of the pointed objects and the objects were created using new, then yes the destructor does appear to be correct.

If the pointed objects were not created with new, or the pointers are not owned by A, then A's destructor should not delete them.

If A does own the pointers, it would be bad design, not to also allocate the objects within the class A. But I assume, that was left out of the example for simplicity.

As a sidenote: If A does own the pointers and deletes them in the destructor, then you should also implement the copy constructor and the copy assignment operator so that they make a deep copy or by making the class non-copyable. Otherwise you'll end up with undefined behaviour if you ever copy an instance.

But as you see in Class B and Class C I go to class D, but there would be an error if I delete D?

Doing that is perfectly fine and there will be no error unless...

... if A owns the pointers to B and C, and B and C own the pointers to D, and D owns the pointers to A, so that they all delete the pointed objects, then there is an ownership cycle at class level. That's still fine unless...

... there is a cycle of pointers. For example, an instance of D (let's call it dee) points to an instance of A that points to an instance of B that points to dee, then you have a cycle and you end up deleting an object whose destructor has already started which results in undefined behaviour.

So, if such pointer cycle is possible, then the ownership cycle must be avoided. But if there can be no pointer cycle, then the ownership cycle may exist.

Upvotes: 0

Peter
Peter

Reputation: 36597

It depends on what the constructor does.

In general terms, it is said that a constructor establishes a class invariant (a set of properties that all member functions can assume is true), all member functions used externally to the class (e.g. public members, protected members that provide services to derived classes) maintain that invariant. The destructor does the reverse of that the constructor does - for example, if the constructor explicitly allocates some resource, the destructor releases it.

If that invariant includes "all elements of b are assigned the result of operator new", then the destructor must release those elements using (the corresponding operator delete, or hand off to some other object that does so. This is commonly described as ownership - class A takes actions that affect the lifetime of other objects, so must clean up.

If that invariant includes "all elements of c are either the NULL pointer, or contain the address of an object supplied from elsewhere" then the destructor should not release those objects. For example, some other code may supply a pointer to a member function of A, and that pointer is simply placed into c. This assumes you know that the lifetime of those objects is managed outside your class A.

Consider, for example, a constructor that does this

 A::A(int num_b, const std::vector<C *> &in_c) : b(0), c(in_c)
 {
     b.reserve(num_b);    // so b doesn't keep on resizing itself below
     for (int i = 0; i < num_b; ++i)
        b.push_back(new B);
 }

The logic of this is that A is allocating all the objects who's addresses are stored in b, but is simply copying pointers from in_c (which are presumably managed elsewhere). So class A is responsible for the lifetime of the objects in b, but not those in c.

We then assume that, if member functions change b or c that the same still applies when As destructor is invoked - all objects in b are managed by class A, but those in c are not. (In other words, we assume the class invariant is maintained).

Then the destructor might look like

 A::~A()
 {
     for (std::vector<B *>::iterator i = b.begin(), end = b.end();
          i != end;
          ++i)
     {
          delete (*i);
     }
 }

Note that we don't need to resize b or c, since std::vector's destructor takes care of cleaning itself up. However, since our class allocated objects and stored their address in b - which std::vector has no knowledge of - the destructor must release them.

Note: the above example works for all versions of C++. Post C++11, the code in the destructor can be simplified, for example using new style loops or auto type deduction.

Upvotes: 0

Michael
Michael

Reputation: 1035

Supposing you are not sure that the pointers in the two vectors in class A are all not NULL, then you can do the following:

~A()
{
  for (int i = 0; i < b.size(); i++)
    delete b[i];
  b.clear(); // Making sure you do not access to deleted pointers

  for (int i = 0; i < c.size(); i++)
    delete c[i];
  c.clear();  // Making sure you do not access to deleted pointers
}

We also clear the vectors, in order to be sure not to access to deleted pointes. Alternatively, if you still need the vectors' sizes, you can just set b[i] = nullptr and c[i] = nullptr in the two loops, without clearing the vectors.

Upvotes: 1

Related Questions