CODEWITHSUNDEEP
c++treedestructor
user6230365
user6230365

Reputation:

C++ destructor for tree structure

I have this tree structure: 

public:
    node(string& const n);
    virtual ~node();
    string get_name() const;
    void set_name(string& new_name);
    int get_nr_children() const;
    node get_child(int i) const;
    void add_child(node child);
private:
    string& name;
    vector<node> children;
};

and my main.cpp looks like this:

int main() {
    string s = "root";
    node r(s);
    string s2 = "left child";
    node ls(s2);
    string s3 = "right child";
    node rs(s3);

    r.add_child(ls);
    r.add_child(rs);

    r.~node();
}

(I know that ~node() is run on all objects anyway at the end of the main funciton, but I want to make sure it is executed on the root r first)

All methods are working fine so far, except for the destructor. This is my first destructor, and I came up with the following recursive try, but don't know why it won't work.

node::~node() {
    cout << "Enter ~node of " << this->get_name() << endl;
    while (this->get_nr_children() != 0) {
        this->get_child(0).~node();
        this->children.pop_back();
    }
    delete this;
    cout << "Leave ~node of " << this->get_name() << endl;
}

The result is an endless output of "Enter ~node of left child"

Upvotes: 0

Views: 863

Answers (3)

Emilio Garavaglia
Emilio Garavaglia

Reputation: 20730

"(I know that ~node() is run on all objects anyway at the end of the main funciton, but I want to make sure it is executed on the root r first)"

This quote is enough to say that everything into this question is based only on misconceptions.

It doesn't even make sense to try to correct: a full rewrite is necessary.

Instead read more about destrcuctors, their invocation and what they are for.

Calling it explicitly does not suppress the implicit call of it. And double destruction is undefined behavior.

Also delete this is something very tricky requiring you must be VERY SURE of what it means.

And accessing class methods (this->...) or data after deleting them ... is only looking for troubles.

Upvotes: 3

Pete Becker
Pete Becker

Reputation: 76245

Ordinarily, a tree uses nodes that are allocated on the free store. That means trafficking in pointers to nodes, and deleting nodes that are no longer in use. The code in the question doesn't follow that model: it allocates the root node on the stack, and stores node objects rather than pointers within each node. With that code, all of the compiler-generated destructors will work just fine. There is no need for any user-defined destructors.

Upvotes: 1

Cheers and hth. - Alf
Cheers and hth. - Alf

Reputation: 145239

This is invalid and yields Undefined Behavior if it compiles:

vector<node> children;

At the point of this data member declaration the class node is incomplete; its size isn't yet known.

You can't use an incomplete type as item type for a standard library container (but you can use pointers).

In the destructor implementation, also

delete this;

yields Undefined Behavior, an infinite recursion calling the destructor which (in this delete expression) calls itself, and so on.

The data member declaration 

string& name;

also has a strong, unpleasant odour about it, but since you're not showing the constructor implementations I can't state categorically 100% that it's wrong.

However, given the rest of the code, the chance of this being correct is infinitesimal. Just use

string name_;

Upvotes: 1

Related Questions