Last year I built a generic list, everything was good. This year I'm adding classes to the list. These classes have lists. Everything is okay until the program ends, then it crashes. If I comment out my list destructor the crash doesn't happen.

So I'm assuming the destructors aren't unraveling in the order I expect. I could just leave out the destructor but... 1) I was taught that if I have a "new" I better have a corresponding "delete" and 2) I'd like to know what I'm doing wrong.

Truncated code vomit follows...

Test program and class...

#include "list.h"

class CCrashTest
{
public:
	CCrashTest();
	~CCrashTest();
	void add(int Value, char A, char B);
	list <char> willCrash;
private:
	int a;
};

CCrashTest::CCrashTest() { };
CCrashTest::~CCrashTest() { };
void CCrashTest::add(int Value, char A, char B)
{
	this->a = Value;
	this->willCrash.add(A);
	this->willCrash.add(B);
}

int main ()
{
	list <CCrashTest> crashTest;
	CCrashTest CT;
	int arrayTest4[5] = {18, 5, 231, 68, 185};
	for (i=0; i<5; i++)
	{
		cout << "Adding " << arrayTest4[i] << " to class" << endl;
		CT.add(arrayTest4[i], 'a', 'b');
		crashTest.add(CT);
	}
	cout << "Deleting class" << endl;
	return 0;
}

And the list class (abbreviated)...

#ifndef LIST_H
#define LIST_H

template <class LISTDATA>
class list
{
protected:
	template <class LISTDATA>
	struct listNode
	{
		LISTDATA key;
		listNode <LISTDATA> *prev;
		listNode <LISTDATA> *next;
	};
public:
	list();
	~list();
	virtual void add(const LISTDATA Value);
	virtual LISTDATA get();
	virtual void remove();
	void clearList();
protected:
	listNode <LISTDATA> *start;
	listNode <LISTDATA> *end;
	int numberOfNodes;
protected:
	void addToStart(const LISTDATA Value);
	void addToEnd(const LISTDATA Value);
	LISTDATA removeFromStart();
	LISTDATA removeFromEnd();
	void destroyList();
};

#endif

// ---------------- CONSTRUCTOR & DESTRUCTOR ----------------------

template <class LISTDATA>
list<LISTDATA>::list()
{
	this->start = NULL;
	this->end = NULL;
	this->numberOfNodes = 0;
}

template <class LISTDATA>
list<LISTDATA>::~list() 
{
	this->destroyList();
}

template <class LISTDATA>
void list<LISTDATA>::destroyList()
{
	while (this->end != NULL)
		this->removeFromStart();
}

// ------------------------- ADD -------------------------------

template <class LISTDATA>
void list<LISTDATA>::add(const LISTDATA Value)
{
	this->addToStart(Value);
}

template <class LISTDATA>
void list<LISTDATA>::addToStart(LISTDATA Value)
{
	listNode <LISTDATA> *nodeToAdd;
	nodeToAdd = new listNode<LISTDATA>;
	nodeToAdd->key = Value;
	if (this->start == NULL)
	{
		nodeToAdd->prev = NULL;
		nodeToAdd->next = NULL;
		this->start = nodeToAdd;
		this->end = nodeToAdd;
	}
	else
	{
		this->start->prev = nodeToAdd;
		nodeToAdd->prev = NULL;
		nodeToAdd->next = this->start;
		this->start = nodeToAdd;
	}
	this->numberOfNodes++;
}

// ------------------------- REMOVE ----------------------------

template <class LISTDATA>
LISTDATA list<LISTDATA>::removeFromStart()
{
	listNode <LISTDATA> *tempNode;
	LISTDATA Value;
	if (this->start == NULL)
	{
		throw exceptions("Error (list.removeFromStart): List is empty");
	}
	else
	{
		tempNode = this->start;
		Value = tempNode->key;
		this->start = tempNode->next;
		if (this->start == NULL)
			this->end = NULL;
		this->numberOfNodes--;
		delete tempNode;
	}
	return Value;
}

Thank you much.

You are calling another function which removes only the "head" node (so called "start") and that's it? The destructor is supposed to remove "all" nodes created in the list. It shouldn't throw any error if your "head" is null in the first place. It simply goes through each node and removes it from the memory (one-by-one) until the list is empty. If the list is empty in the first place, it does nothing.

PS: You are supposed to have a while-loop when you deal with list or linked list.

Edited 5 Years Ago by Taywin: n/a

The destructor calls "destroyList". I separated that out to give me the option of a public "clearList" function which calls the same thing. "DestroyList" is a minimal function. While (line 55) end of list isn't null (ie, there are nodes in the list), remove a node. I figured since I had removeFromEnd / removeFromStart functions tried, tested and true, I'd re-use them.

It's probably a weird way of coding. Or a lazy way. :) I could have put all the functionality into the destructor itself, but I don't think that would have helped me in this case. It does all work... unless <LISTDATA> is a class with a list in it...

there are semantics with spaces in c++ with regards to templates that use angle brackets. try removing the space between list and <char>, and enable c++11 coding standards (sometimes called c++0x). without the full code I can't debug your program, but this little piece of advice I can offer.

without c++0x/c++11 there is I believe an issue with regards to things like deque<list<class blah>> because that >> gets parsed as operator>>()

This article has been dead for over six months. Start a new discussion instead.