Destructor for multiple constructors

Question

I have two constructors in C++ and one destructor. When I use the first constructor for my object the destructor is called to delete A[] which is what I want, but when I used the second constructor I don't need to call destructor but C++ compiler calls it anyways which causes an error. What is the best way to solve this problem?

Tree(int n) {

  A = new int[n];
}

Tree(int data*, int n) {
   A = data;
}

~Tree(){

   delete [] A;
}

Answer 1

Store a flag indicating whether the destructor should call delete or not.

Note that you also need a user-defined copy constructor and copy assignment operator, when manually managing memory.

Answer 2

You need to use the Rule of Three in C++03 or Rule of Five in C++11 to avoid this problem.

And typically it is a bad design, In this case only way to distinguish here whether you have ownership of the dynamically allocated buffer is through a boolean flag member, which you will have to set in the constructor if you are the one allocating memory. And then before deleting in destructor you check for this flag and deallocate only if the flag is set.

Tree(int n) 
{

  A = new int[n];
  OwnFlag = true;
}

Tree(int data*, int n) 
{
   A = data;
   OwnFlag = false;
}

~Tree()
{
  if(OwnFlag)
      delete [] A;
}

The code above is just a demonstration for using the boolean flag, You still need to follow the Rule Of Three. I already posted the link, which helps you understand what it is, I don't post the code here because I do not want you just copy paste(and I don't say you would if i added the code, but you know just in case).

Answer 3

This is just a horribly flawed design imo. After the object is created, you have no idea if you're responsible for the memory or not.

Answer 4

Set a flag that indicates if you own the data in A or not, and check it in the destructor to determine if you should free it.

In general, though, it's good to have clearly defined ownership of objects in C++; making explicit prevents memory leaks. Consider making the class always the owner of the object it contains, and having it be responsible for destroying it regardless of how it was created.

Answer 5

In second constructor you are assigning a pointer to an array. What I suggest you to create a heap (allocate for n elements) in second constructor and copy all elements from parameter (data) into newly created heap.

Answer 6

It might be worth considering using two different classes for this:

struct basic_tree { 
    virtual ~basic_tree();
};

// Warning: incomplete -- at the very least, this almost certainly needs to 
// define its own assignment operator and copy ctor (aka the rule of three), 
// or else declare them private to prevent them (or use `=delete` in C++11).
// 
class owning_tree : public basic_tree { 
     int *A;
public:
     owning_tree(int n) : A(new int[n]) {}
     ~owning_tree() { delete [] A; }
};

class non_owning_tree : public basic_tree {
    int *A;
public:
    non_owning_tree(int *data) : A(data) {}
};

This has both good and bad points. On one hand, once you've created an object of the right type, everything else gets handled automatically by the type system. On the other hand, it means you have to specify the correct type at construction time, not just pass the right type of parameter. It also means most tree operations need to use a pointer or reference to a basic_tree, rather than being able to use tree objects directly. Depending on the situation, that could vary from no problem at all (e.g., if a tree will often be large enough that you're normally going to pass by reference anyway) to a massive problem, such as adding a lot of extra code to sort out the right type at creation time.