2

I have a function that creates a binary tree(*build_tree)* (Huffman).

I also need a function that free's the memory that build tree allocated.

It's my first time working with binary trees so I'm kind of confused. Should I create a loop that goes through each node in the tree and deletes it? Should I consider if the node is a leaf or a parent node?

void free_memory(NodePtr root)
{
    delete root;
}

struct HuffmanNode
{
   //some stuff
    HuffmanNode *left;
    HuffmanNode *right;
};

Would appreciate it if someone could help me get started :)

user3265963
  • 273
  • 3
  • 11
  • 18

2 Answers2

2

If you use smart pointers the problem will solve itself. If each node contains a private SP to it's children and you delete a node all it's children will also be freed. Obviously your class destructor, which will get called by the SP when it cleans up will need to free any other non RIIA allocated resources if any exist.

class Node
{
private:
   std:unique_ptr<Node> left;
   std:unique_ptr<Node> right;
}

I'm using std::unique_ptr<> here because I'm assuming that these are private and not exposed to other parts of your program. If you want other things to reference nodes using these pointers then you should use std::shared_ptr<>.

If you're not using SP then the class destructor needs to do the work itself and you have to be much more careful about memory leaks. Each class destructor deletes its children, which in turn will call the destructor in each child.

class Node
{
private:
  NodePtr left;
  NodePtr right;

  ~Node()
  {
     delete left;
     delete right;

     // Delete any other resources allocated by the node.
  }
}

You can also do as @OldProgrammer suggests and traverse the tree bottom up deleting nodes as you go. Remember you have to do this bottom up. If you did it top down then you would loose the reference to the (as yet) undeleted child nodes and leak memory. As you can see the code for recursive deletion (as referenced in @unluddite's answer) is a lot more complex.

There is a memory overhead for doing (anything) recursively. See: Is a recursive destructor for linked list, tree, etc. bad?. If your tree is very large then you should consider this and test accordingly.

My recommendation would be for the first solution if you are OK with using SP.

Community
  • 1
  • 1
Ade Miller
  • 13,575
  • 1
  • 42
  • 75
  • No need to check for null before a delete it is well defined to do nothing (and looks a lot neater). – Martin York Feb 17 '14 at 16:36
  • I have both left and right in my struct(I edited the OP). I use the as pointers to the children. Could something like this work: void free_memory(NodePtr root) { if(root->left != NULL) delete root.left; if(root->right != NULL) delete root.right; }void free_memory(NodePtr root) { if(root->left != NULL) delete root.left; if(root->right != NULL) delete root.right; } – user3265963 Feb 17 '14 at 16:37
  • No pointint in setting left and right to NULL. The objet is about to go out of scope. This is both a waste of time and a way to hide real issues when testing the code. – Martin York Feb 17 '14 at 16:40
  • @Loki Astari Good catch. Fixed the code. I'm assuming the recommendation to set the pointer to null post-delete is still a good one. – Ade Miller Feb 17 '14 at 16:40
  • @user3265963 no. if you delete the root::left and root::right pointers first how do you get to the child nodes to delete them? Unless you're solution also declares a destructor. In which case you can just delete root;. BTW: Code in comments is hard to read. You might want to update your answer instead. – Ade Miller Feb 17 '14 at 16:42
  • @Loki Astari OK got that too. Thanks for your feedback. As you can tell I don't use delete very often :) – Ade Miller Feb 17 '14 at 16:44
2

If you implement a post-order tree traversal and delete the node and data at the process step, you will ensure that each node of your tree is visited and the data deleted.

You can see a recursive and iterative example here with the relevant code reproduced below.

Recursive solution:

void postOrderTraversal(BinaryTree *p) {    
    if (!p) return;
        postOrderTraversal(p->left);
        postOrderTraversal(p->right);

        // this is where we delete
        delete p->data;
        delete p;
    }

One possible iterative solution:

void postOrderTraversalIterative(BinaryTree *root) {
    if (!root) return;
    stack<BinaryTree*> s;
    s.push(root);
    BinaryTree *prev = NULL;
    while (!s.empty()) {
        BinaryTree *curr = s.top();
        if (!prev || prev->left == curr || prev->right == curr) {
            if (curr->left)
                s.push(curr->left);
            else if (curr->right)
                s.push(curr->right);
        } else if (curr->left == prev) {
            if (curr->right)
                s.push(curr->right);
        } else {
            // this is where we delete
            delete curr->data;
            delete curr;
            s.pop();
        }
        prev = curr;
    }
}
Eugene S
  • 3,092
  • 18
  • 34
  • if(root == NULL) { return; } delete root; – user3265963 Feb 17 '14 at 16:33
  • Could it be implemented as something like that (in the comment above)? Does the function the go through this step until it hits NULL? – user3265963 Feb 17 '14 at 16:35
  • Sure, but where's the traversal step? You need to visit the left and right sub-trees before you delete the root. – Eugene S Feb 17 '14 at 16:37
  • Linking to code external to the site is not always a good idea. This answer may live much longer then some arbitrary blog and when the linked article is delete this answer becomes useless. Copy the relevant (part) into the answer. – Martin York Feb 17 '14 at 16:41
  • Cheers! The first one worked for me. Except I did not have delete p->data; Can you use delete on 'non pointer' elements? – user3265963 Feb 17 '14 at 17:10
  • @user3265963 No, `delete` should be called on pointer types that are pointing to heap-allocated memory (allocated with `new` or `malloc` or their variants). – Eugene S Feb 17 '14 at 18:56