Avoid infinite recursion in destructor

7

As part of an exercise my university has tasked me with, I have written a small Graph implementation, following this header.

class Node {

private:
    std::string name;
    std::vector<Node*> children;

public:
    Node(const std::string& name="");
    virtual ~Node();

}

When writing code for the destructor ~Node(), I noticed that my implementation fails when the graph contains a cycle. This is my implementation so far, which obviously doesn't work if the graph contains a cycle.

Node::~Node() {
    for (Node* n : children) {
        delete n;
        n = NULL;
    }
    children.clear();
}

I am uncertain as to how I would most elegantly write a destructor that can handle cycles in the graph?

Please note that I was specifically tasked to write a recursive destructor. Thank you for your answers!

Share
Improve this question
12
  • 1
    1. Don't delete the children in ~Node. 2. Let a higher level function/class manage the destruction of all the Nodes in the graph. – R Sahu 16 hours ago
  • 4
    It looks like you have an ownership problem. You have shared ownership (multiple objects can have pointers to the same object, and any of them may be responsible for deleteing it) as well as potentially circular ownership. This is not a trivial problem. You should change your ownership scheme, store your Nodes in some sort of Graph object, and have only the Graph object be responsible for destroying every Node it contains. Remember to use unique_ptr instead of raw pointers (std::unique_ptr<T> vs T*) when a pointer own an object. – François Andrieux 16 hours ago
  • 5
    "Please note that I was specifically tasked to write a recursive destructor." A recursive destructor is a very very poor solution for this kind of problem. – François Andrieux 16 hours ago
  • 1
    @FrançoisAndrieux "A recursive destructor is a very very poor solution for this kind of problem" not if the problem is to teach ownership and the perils of recursive destructors. The university is a safe place to experiment with code and understand choices. You want to hit these problems now, not on your job where you have real responsibilities and real consequences. – bolov 16 hours ago
  • 2
    @bolov It's still a poor solution for the problem, even if it is a good example of a poor solution. And it's good that OP learns of this now, in case they wouldn't learn it in the university where poor solutions don't have real consequences. – eerorika 16 hours ago

Comments

Popular posts from this blog

Meaning of `{}` for return expression

Get current scroll position of ScrollView in React Native

flutter websocket connection issue