0

When destroying(when objects is out of scope) General Tree is it necessary to traverse each node and deleting them like you do with doubly linked list? The General Tree I am writing is a circular so that insertion can be done in constant time. It would be much helpful if someone could help me fix the error. The destructor below currently causes stack overflow( I think it is because of the tree being circular).

Here are the 2 destructors

~Node()
         {
        if(left){delete left;}
                if(next){delete next;}
                if(parent){delete parent;}
          }

for General tree

 ~Gen()
       {
           if (head)
                 delete head; //call destructor on node
                 head = nullptr;
                 m_size = 0;
        }

This is the node class

class Node {
public:
    typedef Node* nodePtr;
    int data;
    //Left child- right sibling implementation
    nodePtr left, next, parent;
    int rank; //will be used for merging.
~Node()
         {
               if(left){delete left;}
                if(next){delete next;}
                if(parent){delete parent;}
          }
        
private:
    
    Node & operator =(const Node&);
};

this is the tree that uses the node above

class Gen{
 public:
      typedef Node* nodePtr;
       Gen():m_size(0),head(0){}
       ~Gen()
       {
           if (head)
                 delete head; //call destructor on node
                 head = nullptr;
                 m_size = 0;
        }
        void push(int val)
        {
        nodePtr newNode = new Node;
        
        newNode->data = val;
        newNode->rank = 0;
        newNode->left = newNode->next = newNode->parent = 0; //set all pointers to null
        insertRoot(newNode); //call the inserthelper
        ++m_size;
        }
        //other functions (deleteMin, decreaseKey etc)
private:
int m_size;
nodePtr head;
nodePtr insertRoot(nodePtr newNode)
{
   //create a circular link
   if (!head)
    {
        head = newNode;
        newNode->next = newNode;
    }
    else
    {
        newNode->next = head->next;
        head->next = newNode;
        if (newNode->data < head->data)  //min heap (lazy insert)
            head = newNode;
    }
}
};

Ghost001
  • 43
  • 7
  • `if(parent){delete parent;}` You likely don't want that; it leads to double destruction. A parent owns its children and is responsible for deleting them, not the other way round. – Igor Tandetnik Mar 01 '21 at 04:26
  • As this is built, there is no O(1) mechanism for expunging the entire tree. It needs to be obliterated the same way it was assembled; brick by brick. – WhozCraig Mar 01 '21 at 04:30
  • And yes, the fact that `node->next` links form a circular list also leads to double destruction, once you go full circle and come back to the node already being destroyed. You probably want to implement the destruction iteratively rather than recursively. – Igor Tandetnik Mar 01 '21 at 04:32

2 Answers2

1

You will need to call the destructor of each of the members of the tree in some way, either by traversing the tree in recursive manner (like your implementation) or by iterative methods.

Simplest iterative approach would be to use a stack and add the elements as you go down the tree, and pop the elements to delete them when you go up the tree. Check more on this approach in this post

So in your specific case, the General Tree will have a distractor to travel trough the tree to destroy the nodes, and you don't need the node destructor. The destructor will look something like:

#include <stack>
...
....
~Gen() {
    std::stack<Node*> s;
    s.push(head);
    Node* current;
    while (!s.empty()) {
        current = s.top();
        s.pop();
        if (current->left != nullptr) {
            s.push(current->left);
        };
        if (current->next != nullptr) {
            s.push(current->next);
        }
        delete current;
    }
    head = nullptr;
}

So, as you are progressing down the tree, you add the children to the stack and you delete the parent. Once the stack is empty, all nodes in the tree are deleted.

jordanvrtanoski
  • 5,104
  • 1
  • 20
  • 29
  • OP didn't quite describe their data structure well, but they seem to be implementing left-child right-sibling tree in which case `left, next` is consistent and conventional naming. – eerorika Mar 01 '21 at 05:15
  • In such case the name `General Tree` is misleading, however, I remove the comment since this is not the essence of the question nor the answer. – jordanvrtanoski Mar 01 '21 at 05:36
  • I added a flag to indicate whether the Node was visited and deleted it by post order traversal. I still like your method better so I will change it. – Ghost001 Mar 01 '21 at 06:30
  • As `std::stack` has to allocate and may throw `std::bad_alloc`, this proposed solution violates [C.36: A destructor must not fail](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rc-dtor-fail). – Tim Rakowski Mar 03 '23 at 16:12
0

Is it necessary to traverse a General Tree for destroying the tree (destructor)?

Yes, it is necessary to visit every node in order to delete them (or at least every branch since you can delete the leaves from branches).


I assume that parent of node X points to a such node whose child X is. A problem with your implementation of the node destructor is that destructor of parent deletes its child, and its child deletes the parent which deletes the child which deletes the parent which deletes the child which deletes the parent which deletes the child... Can you spot the problem? The recursion is endless. Besides, the lifetime of the parent has already ended, so the child attempting to delete the parent results in undefined behaviour.

The solution to this is simple: Don't delete parent. If you start from root and delete children, and if parent always points to a node that is already being deleted, then you visit all nodes of a tree as desired.


Another problem is that your tree isn't balanced, so the depth of your tree may in worst case be linear in relation to the number of elements. This causes the depth of recursion of your destructor to grow linearly (in worst case) which may lead to stack overflow even if the infinite loops weren't a problem.

You shouldn't use recursion to destroy trees that aren't balanced. You should use trivial destructor for the node, and implement iterative destructor for the tree.

A good algorithm to destroy an unbalanced binary tree is to rotate one subtree until it is null, then delete root and repeat with the other child node. Example (entirely untested, may be buggy):

while (head) {
    if (head->left) {
        // rotate
        Node* temp_left = head->left;
        head->left = head->left->next;
        temp_left->next = head;
        head = temp_left;
    } else {
        Node* temp_next = head->next;
        delete head;
        head = temp_next;
    }
}

Tree I am writing is a circular

This also breaks your implementation of the destructor since when you reach a "leaf" pointing back to the root, you will enter similar infinite loop and undefined behaviour described above.

Unfortunately, this property also breaks my suggested iterative destructor. I don't have time now to figure out a good solution though. An idea is to combine it with a cycle detection algorithm.

eerorika
  • 232,697
  • 12
  • 197
  • 326