Thread: Destroying a linked list

Hi all!

I have a destructor implemented below. The problem is, it seems as if it deletes all of the items in the list properly but when the program gets to the final line of code, which is just the return statement in main, it throws an access violation. Can anyone help figure out why this is?

Note: I can't put the whole program on here, because it's well over 800 lines. So, I'll just post an excerpt or two.

Each "student" is a node. With data and a pointer (link) to the next node. I also have head and tail nodes which are the beginning and end of the list. I have double checked the constructor to make sure the tail points to NULL.

Code:

singlyLinkedList::~singlyLinkedList()
{
	student *temp;		//Used for traversing list
	student *swap;		//Used for holding values while deleting list
	if (head != NULL && tail != NULL)
	{
		swap = head;	
		
		while (swap != NULL)
		{
			temp = swap;
			swap = swap->link;
			delete temp;
		}
		head = NULL;
		tail = NULL;
	}
}

Thanks!!

EDIT:
As I mentioned, it seems as if the list is destroyed correctly. However, I noticed when the program reaches the end of main, it calls the destructor again, and the pointers for head and tail are both bad (not null).... With breakpoints, I've noticed that students (in main, reprinted below) gets destroyed before the cout statement, and then again at the end of the program... Does anyone know why it might be happening in that order?

both students and classLists implement this linked list.

Code:

int main()
{
	//Initialize the list of classes
	string classNames[22] = { "CSCE101", "CSCE 101L", "CSCE 150E", "CSCE150EL",
							  "CSCE150EM", "CSCE150EML", "CSCE 155", "CSCE155H",
							  "CSCE 156", "CSCE 230", "CSCE 230L", "CSCE 235",
							  "CSCE 251", "CSCE 310", "CSCE 322", "CSCE 361",
							  "CSCE 351", "CSCE 451", "CSCE 423", "CSCE 428",
							  "CSCE 486", "CSCE 487" };

	singlyLinkedList classLists[22];
	singlyLinkedList students;

	for(int i=0; i< 22; i++)
	{
		classLists[i].setClassName(classNames[i]);
	}
	students.setClassName("Student List");
	parseData(classLists, students, 22);
	
	for (int i=0; i < 22; i++)
		cout << classLists[i];

	return 0;
}

If anyone wants to take a look at the whole code, I've included it as attachments.

Originally Posted by m37h0d

you don't have a linked list, you have a static array. classnames is declared on the stack and should not be destroyed by you.

Code:

string *classnames = new string[22];

needs to be destroyed by you.

This is correct, but I highly recommend you use a vector of strings instead. The only downside is that until C++0x is final, you can't initialize the vector with an initializer list as you can with strings/arrays.

But that downside is tiny when you consider the fact that the vector will do its own cleanup.

2) Pass by reference (good ole "&", anywhere you don't want the copy constructor to be invoked).

And to catch accidental attempts at pass by value, declare copy constructor and operator= private (or inherit from a suitable base class like boost::noncopyable).