Destructor of the object containing the vector of the specified object
I have a little problem with the destructor. In its current state, it creates a segfault. Note that the destructor is implemented and is never mentioned anywhere. Segfault appears regardless of where the breakpoints are.
Here is the destructor:
Graph::~Graph() {
while(!children.empty()) {
for(vector<Graph*>::iterator itr = children.begin(); itr != children.end(); ++itr) {
delete *itr;
}
children.clear();
delete parent;
delete tab;
}
}
I also made a variation like this, with no better results:
Graph::~Graph() {
while(!children.empty()) {
for(unsigned i = 0; i < children.size(); i++)
{
delete children.at(i);
}
children.clear();
delete parent;
delete tab;
}
}
Here are the class declarations:
class Graph
{
private :
Graph* parent;
vector<Graph*> children;
Board* tab;
public :
Graph(Board);
Graph(Board, Graph*);
~Graph();
void AddNode(Board&);
// Graph& BFS(Graph&);
Graph& operator=(Graph source);
vector<Graph*>& getchildren();
Graph* getparent();
Board* gettab();
};
class Board {
private :
int** tab;
int nbline;
int nbcolumn;
Position emptyspot;
public :
Board();
Board(int, int, Play&);
Board(int, int);
Board(const Board&);
Board(int, int, ifstream&);
~Board();
void setValue(Position&, int);
void setNbline(int m);
void setNbcolumn(int n);
int getValue(Position&);
int getNbline();
int getNbcolumn();
int getEmptyline();
int getEmptycolumn();
void setEmptySpot(Position&);
Position& getEmptySpot();
Board& operator=(Board& source);
};
Board::~Board()
{
for(int i = 0; i < this->nbline; i++)
{
delete tab[i];
}
delete tab;
}
I'm not very comfortable and very inexperienced with the debugger, so I don't know how to use it correctly. The call stack point on this line is stl_vector.h:
/**
* Returns a read-only (constant) iterator that points one past
* the last element in the %vector. Iteration is done in
* ordinary element order.
*/
const_iterator
end() const _GLIBCXX_NOEXCEPT
{ return const_iterator(this->_M_impl._M_finish); }
I don't know what these lines mean, to be honest.
The call stack also displays the while while line in the debugger with a note: Graph :: ~ Graph (this = 0x90909090, __in_chrg = optimized). I also specify 3 times the delete * itr line (with the same entry).
So my question is, how can I destroy the Graph object?: '(
EDIT: After further experimentation, the segfault disappears when I remove the only line code that adds things to the vector. This is the method. I'll add that the values in the vector are always names (shouldn't be).
void Graph::AddNode(Board& tablo)
{
Graph tmp(tablo, this);
Graph* newChild = &tmp;
children.push_back(newChild); // if i commend this, no segfault
}
I don't know if these are two different problems, or if push_back is causing the destructor to malfunction. I think it is unrelated, I expected the segfault to go away (of course, the destructor has no problem destroying the tree if the tree only got a node).
EDIT2: This code doesn't create any segfault, but it doesn't really destroy all vectors in vectors, right? I think this is not because erasure will just destroy the pointers, not themselves.
while(!children.empty()) {
children.erase(children.begin(),children.end());
delete parent;
delete tab;
}
In addition, sometimes the program works well, but does not stop at the end of execution. Debbuger seems to find nothing
EDIT: as ask, Graph copy constructor:
Graph :: Graph (const Graph & source) {* this = source;}
Graph& Graph::operator=(Graph source)
{
if(this!=source)
{
this->parent = source.parent;
this->tab = source.tab;
// this->seen = source.seen;
while(!source.children.empty()) {
for(unsigned i = 0; i<source.children.size(); i++) {
this->children.at(i) = source.children.at(i);
}
}
}
return *this;
}
source to share
The problem is this:
delete parent;
Your loop enforces the semantics to which it belongs Graph
. But by adding this line, you additionally add semantics to which Graph
its parent belongs. You cannot have it both ways. This way, your child will remove the parent when it is removed. Removing delete
should fix your problem.
Better yet would be to simply express ownership directly in terms of member variables:
struct Graph {
std::vector<std::unique_ptr<Graph>> children;
};
This way everything will be removed correctly, even if you don't have to write a destructor! See the Zero Rule
source to share
I agree with @Barry that the line
delete parent;
shouldn't be in the destructor.
Also, the destructor can be cleaned up a bit.
You have a challenge
delete tab;
inside the loop while
. The call must be present regardless of whether there are children or not. Is not it?
You can also remove the cycle entirely while
.
Graph::~Graph() {
// Just loop over the children and delete them.
// If there are no children, the loop is a no op.
for(vector<Graph*>::iterator itr = children.begin(); itr != children.end(); ++itr) {
delete *itr;
}
// No need for this.
// The vector will be deleted anyway.
// children.clear();
// Delete tab regardless of the number of children.
delete tab;
}
source to share
The AddNode function seems to be wrong. The tmp object will be destroyed when the function completes, and then the pointer in the vector becomes invalid and will cause a problem on a subsequent call to delete.
Use a new one instead. Maybe like this:
void Graph::AddNode(Board& tablo)
{
Graph* newChild = new Graph(tablo, this);
children.push_back(newChild);
}
source to share
I will summarize my modifications and their consequences.
I changed the destructor as @R Sahu and Barry said.
Graph::~Graph() {
for(vector<Graph*>::iterator itr = children.begin(); itr != children.end(); ++itr) {
delete *itr;
}
delete tab;
}
I changed addNodes as Nielsen advised (so does Dieter Lucking):
void Graph::AddNode(Board& tablo)
{
Graph tmp(tablo, this);
Graph* newChild = new Graph(tablo, this);
children.push_back(newChild);
}
And as Paul Mackenzie asked, here is my = operator and copy constructor for Graph.
Graph::Graph(const Graph& source) : parent(source.parent), tab(source.tab) {
while(!source.children.empty()) {
for(unsigned i = 0; i<source.children.size(); i++) {
this->children.at(i) = source.children.at(i);
}
}
}
Graph& Graph::operator=(Graph source)
{
this->parent = source.parent;
this->tab = source.tab;
// this->seen = source.seen;
return *this;
}
This makes me realize that the = operator does not copy the vector: '(
Now how the execution is done. Segfault is still here, but the callstack has changed a lot. I only have 2 lines in the call stack, not 5/6. The column told me there is a problem in
Board* Graph::gettab(){return this->tab;}
When I do this to see what is in the vector graph.children:
for(vector<Graph*>::iterator itr = children.begin(); itr != children.end(); ++itr)
{
tmp = (*itr)->gettab();
Board& test = *tmp; // it print(Board&) so i just create this. I'll create print(int**) later
print(test);
}
If I put these lines in the comments, segfault is still here. The column quotes me 3 lines in stl_iterator.h (do you need these?) And both the for loop and delete * itr lines in the destructor.
EDIT:
@nielsen You asked for my main.
int main()
{
int lines, columns;
Play game;
Play& jeu = game;
srand(time(NULL));
cout << "+++++++++++++++++++++++++++++" << endl;
cout << "+++ SOLVER +++" << endl;
cout << "+++++++++++++++++++++++++++++" << endl;
cout << endl;
cout << endl;
cout << "Random Board \nLignes : ";
cin >> lines;
cout << "\n Colonnes : ";
cin >> columns;
Board randomTab(lines, columns, jeu);
print(randomTab);
trace(randomTab);
cout << "________________" << endl;
Graph tree(randomTab); /// Call stack point to this !
Board temporary(randomTab);
Board& temp = temporary;
Board* tmp = NULL;
bool controls = false;
vector<int> directions {72,75,77,80};
for(vector<int>::iterator itr = directions.begin(); itr != directions.end(); ++itr)
{
temp = randomTab;
controls = jeu.moves(temp,*itr);
if(controls)
{
cout << *itr << endl;
tree.AddNode(temp);
print(temp);
// trace(temp);
controls = false;
}
}
cout << "__________________" << endl;
vector<Graph*> children = tree.getchildren();
/* for(vector<Graph*>::iterator itr = children.begin(); itr != children.end(); ++itr)
{
tmp = (*itr)->gettab();
Board& test = *tmp;
print(test);
cout << "test";
trace(temporary);
}*/
return 0;
}
Graph::Graph(Board source) : parent(NULL), tab(&source) {} // constructeur pour racine
Graph::Graph(Board source, Graph* grap) : parent(grap), tab(&source) {} // constructeur pour nouvelle feuille
Graph::Graph(const Graph& source) : parent(source.parent), tab(source.tab) { // constructeur par copie
while(!source.children.empty()) {
for(unsigned i = 0; i<source.children.size(); i++) {
this->children.at(i) = source.children.at(i);
}
}
}
source to share
Someone asked me how the Council is created. In this current use case:
Board::Board(int m, int n, Play& jeu) : tab(new int*[m]), nbline(m), nbcolumn(n), emptyspot(n-1,m-1){
int x(1);
for (int i = 0; i < m; ++i){
tab[i] = new int[n];
for(int j = 0; j < n; ++j) {
tab[i][j] = x; x++;}}
tab[n-1][m-1]=0;
x=0;
while (x!=1000)
{
int numbers[] = { UP, DOWN, LEFT, RIGHT };
int length = sizeof(numbers) / sizeof(int);
int randomNumber = numbers[rand() % length];
jeu.moves(*this, randomNumber);
x++;
}
}
/// copy constructor
Board::Board(const Board& origin): tab(NULL), nbline(origin.nbline), nbcolumn(origin.nbcolumn), emptyspot(origin.emptyspot)
{
this->tab = new int*[this->nbline];
for (int i = 0; i < this->nbline; ++i)
{
this->tab[i] = new int[this->nbcolumn];
for (int j = 0; j < this->nbline; ++j)
{
this->tab[i][j] = origin.tab[i][j];
}
}
}
Board::~Board()
{
for(int i = 0; i < this->nbline; i++)
{
delete tab[i];
}
delete tab;
}
Board& Board::operator=(Board& source)
{
if (this != &source)
{
Position pos(0,0);
Position& p=pos;
this->setNbline(source.getNbline());
this->setNbcolumn(source.getNbcolumn());
this->setEmptySpot(source.getEmptySpot());
for (int i =0; i < source.getNbline(); i++)
{
for(int j=0; j < source.getNbcolumn(); j++)
{
p.setposition(i,j);
setValue(p,source.getValue(p));
}
}
}
source to share