2

I am trying to implementing a SegmentTree class to be used in another Solution class.

In class SegmentTree, the destructor is implemented as follows:

~SegmentTree() {
    destruct(root);
};

void destruct(Node* root) {
    if (!root) return;
    
    destruct(root->left);
    destruct(root->right);
    delete root;
    root = NULL;
}

Then, in class Solution, we have

class Solution {
private:
    SegmentTree* tree;

public:
    Solution(vector<int>& nums) {
        tree = new SegmentTree(nums);
    }

    //Question: how shall ~Solution() be implemented to free memory of the pointer tree?
};

The question:

  1. Do we need to implement destructor to delete tree pointer in Solution class?
  2. If yes, how shall that be implemented?
thinkdeep
  • 945
  • 1
  • 14
  • 32
  • 1
    See [Rule of Three/Five](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) and ask yourself if these should really be raw pointers. You don't need to implement a destructor at all if your type clearly reflects your intentions, which raw pointers never do. Alternatively, if you're trying to learn memory manipulation, do it in C and forget about destructors entirely. – Silvio Mayolo Jul 22 '21 at 18:31
  • Why are you using `SegmentTree* tree;` and not `SegmentTree tree;`? – NathanOliver Jul 22 '21 at 18:32
  • `SegmentTree tree` will have memory allocated in the stack? If the tree is very large, then, will it cause stackoverflow for `class Solution`? – thinkdeep Jul 22 '21 at 18:34
  • 1
    No, it will have automatic memory, which means it'll be stored the same place as `Solution`. Stack space is often used for local variables with automatic storage, but making something a scalar value in a class doesn't magically put it on the stack if it wasn't already. – Silvio Mayolo Jul 22 '21 at 18:35
  • Looks like your segment tree does dynamic allocation internally so `sizeof(SegementTree)` should be fairly small. – NathanOliver Jul 22 '21 at 18:35
  • @thinkdeep I'm pretty sure it does not matter where `SegmentTree` is allocated: I assume the type itself is pretty small and most of the memory is dynamically allocated for the `Node`s. You can have objects on the stack that manage the memory on the heap... The one issue I see that is independent of whether you store the object on the heap or stack is that you could get a stackoverflow in the destructor; this could possibly be avoided by using a while loop and restructuring the tree to become something like a linked list during destruction... – fabian Jul 22 '21 at 18:41

2 Answers2

5

Do we need to implement destructor to delete tree pointer in Solution class?

If you use new to create an object, you need to delete the object when you are done using it. So yes, Solution needs a destructor to delete the tree that it new's (just as SegmentTree needs a destructor to delete the Node's that it new's).

If yes, how shall that be implemented?

Like this:

class Solution {
private:
    SegmentTree* tree;

public:
    Solution(vector<int>& nums) :
        tree(new SegmentTree(nums))
    {
    }

    ~Solution() {
        delete tree;
    }
};

Which you can automate by using std::unique_ptr, eg:

class Solution {
private:
    std::unique_ptr<SegmentTree> tree;

public:
    Solution(vector<int>& nums) :
        tree(std::make_unique<SegmentTree>(nums))
        //tree(new SegmentTree(nums))
    {
    }
};

Either way, do be aware of the Rule of 3:

If a class requires a user-defined destructor, a user-defined copy constructor, or a user-defined copy assignment operator, it almost certainly requires all three.

So, you should also implement (or disable) the copy constructor and copy-assignment operator, eg:

class Solution {
private:
    SegmentTree* tree;

public:
    Solution(vector<int>& nums) :
        tree(new SegmentTree(nums))
    {
    }

    Solution(const Solution &src) :
        tree(new SegmentTree(*(src.tree)))
    {
    }

    ~Solution() {
        delete tree;
    }

    Solution& operator=(const Solution &rhs) {
        if (&rhs != this) {
            Solution tmp(rhs);
            std::swap(tree, tmp.tree);
        }
        return *this;
    }
};

Though, you might consider simply getting rid of new to begin with and just let the compiler manage the SegmentTree object for you:

class Solution {
private:
    SegmentTree tree;

public:
    Solution(vector<int>& nums) :
        tree(nums)
    {
    }
};
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
4

There must be exactly one delete for every new or there will be a memory leak. You can write a destructor like this:

class Solution {
private:
    SegmentTree* tree;

public:
    Solution(vector<int>& nums) {
        tree = new SegmentTree(nums);
    }
    ~Solution() { 
        delete tree;
    }
};

However, you need to read about the rule of 3/5 (What is The Rule of Three?), because when a class manages a resource a constructor and destructor are not sufficient. For example copying the above Solution will cause problems (because the compiler generated assignment and copy constructor aren't doing the right thing). When possible you should strive to follow the rule of 0, for example by not using a pointer in the first place:

class Solution {
private:
    SegmentTree tree;

public:
    Solution(vector<int>& nums) : tree(nums) {}
};

If the member must be dynamically allocated you should use a std::unique_ptr (and again rule of 0) but never a raw pointer.

463035818_is_not_an_ai
  • 109,796
  • 11
  • 89
  • 185