How to delete a pointer stored within a node?-CodePudding

I'm trying to set up a binary tree comprised of nodes that hold pointers to objects, but in my "clear tree" function I come across a read access violation when trying to free memory at the pointer within the node. Why is there no exception thrown when I free memory at the root pointer, but there is at the int pointer within the node?

Exception thrown: read access violation. it was 0x2.

class Tree {
private:
    struct Node {
        int* val = nullptr;
        Node* right = nullptr;
        Node* left = nullptr;
    };
    Node* root = nullptr;

public:
    bool Insert(int* num);
    void Empty();
    bool isEmpty() const;
};

void Tree::Empty()
{
    while (!(root == nullptr)) // Just handling the simplest case for now
    {
        if (root->left == nullptr && root->right == nullptr)
        {
            delete root->val;   // Read access violation 
            delete root;
            root = nullptr;
            break;
        }

        [...]
    }
}

bool Tree::Insert(int* num)
{
    Node* insertion = new Node;
    int* temp = new int(*num);
    insertion->val = temp;

    if (root == nullptr)
    {
        root = insertion;
        return true;
    }

    Node* c_node = root;

    while (true)
    {
        if (*temp == *c_node->val)
        {
            delete temp;
            delete insertion;
            return false;
        }

        if (*temp > *c_node->val)
        {
            if (c_node->right != nullptr)
            {
                c_node = c_node->right;
                continue;
            }

            c_node->right = insertion;
            return true;
        }

        if (c_node->left != nullptr)
        {
            c_node = c_node->left;
            continue;
        }

        c_node->left = insertion;
        return true;
    }
}

int main()
{
    int a = 2;

    Tree my_tree;

    my_tree.Insert(&a);
    my_tree.Empty();
}

I'd appreciate any feedback!

CodePudding user response：

I would suggest starting with making Node responsible for its own content:

struct Node {
    Node(int *val) : val(new int(*val)) { }
    int* val = nullptr;
    Node* right = nullptr;
    Node* left = nullptr;
    ~Node() { delete val; }
};

Having done this, we can simplify the code for Empty (and Insert) a bit by letting it deal with the value it's storing, so the fragment of Empty you've implemented so far ends up something like this:

void Tree::Empty()
{
    while (!(root == nullptr)) // Just handling the simplest case for now
    {
        if (root->left == nullptr && root->right == nullptr)
        {
            delete root;
            root = nullptr;
            break;
        }
    }
}

As for making this implementation work for a tree with more than one node, I'd probably do it recursively:

void Tree::Empty(Node *node)
{
    if (node == nullptr)
        return;

    Empty(node->left);
    Empty(node->right);

    delete node;
}

I'd probably also define a dtor for Tree, so the user doesn't need to explicitly call Empty (in fact, I'd probably make Empty private, so the outside world can't call it at all, but that's a separate question).