Haozhuo Huang Haozhuo Huang - 3 months ago 9
C++ Question

Circular data dependency destructor

I am now designing my own graph class with adjacency list. I finished most of steps except for the destructor.

This is my Vertex class:

struct Vertex{
public:
Vertex(){m_name="";}
Vertex(string name):m_name(name){}
~Vertex(){
cout << "vertex des" << endl;
for(int i = 0; i < m_edge.size(); i++){
delete m_edge[i];
m_edge[i] = nullptr;
}
}
string m_name;
vector<Edge*> m_edge;
};


This is my Edge class:

struct Edge{
public:
Edge() : m_head(nullptr), m_tail(nullptr) {m_name="";}
Edge(string name) : m_name(name), m_head(nullptr), m_tail(nullptr) {}

~Edge(){
cout << "Edge des" << endl;

delete m_head;
m_head = nullptr;
delete m_tail;
m_tail = nullptr;
}

string m_name;

Vertex* m_head;
Vertex* m_tail;
};


However, I noticed when destructor is called, both 2 classes actually call each other's destructor so this gives me an infinite loop. Is this design problematic? If not, is there is any way to fix this destructor issue? Thanks!

Answer

However, I noticed when destructor is called, both 2 classes actually call each other's destructor so this gives me an infinite loop. Is this design problematic?

Your current design is problematic indeed. Dynamically allocated should only be deleted by their respective owners. Typically, the owner is whoever created the object and typically, there is exactly one owner. If there are more than one object, then the ownership is shared. Shared ownership requires a mechanism - such as reference counting - to track the current number of owners.

Judging by the destuctors, your vertices appear to be "owned" by multiple edges and the edges appear to be owned by multiple vertices. If that wasn't the case, then your graphs would be quite boring. But you haven't implemented any form of ownership tracking.

I recommend using a simpler design where edges don't own vertices nor do vertices own edges. Both of them should be owned by a parent object perhaps called Graph.