Can someone find the error in this destructor?

I don't know what's going on here, but its going to crap everytime. I'm just trying to clear all the memory like a good programmer.

Code:

#include "stack.h"
#include <cstddef>

Stack::Stack()
{
}

Stack::~Stack()
{
	//while (!isEmpty())
	//	pop();

	StackNode *temp;

	while (topOfStack != NULL)
	{
		temp = topOfStack;
		topOfStack = topOfStack->next; // Seems to be a problem here
		delete temp;
	}
}

doesn't the ~ go beforehand like ~stack::stack

Originally posted by Extol
doesn't the ~ go beforehand like ~stack::stack

no it doesnt
it goes
return type classname::function name
like

Code:

int stack::sum(int x, int y);

or
cstack::cstack(); //constructors dont have return types

this may or may not be it and im new at c++ but shouldnt it be:

Code:

#include "stack.h"
#include <cstddef>

void Stack::Stack()
{
}

void Stack::~Stack()
{
	//while (!isEmpty())
	//	pop();

	StackNode *temp;

	while (topOfStack != NULL)
	{
		temp = topOfStack;
		topOfStack = topOfStack->next; // Seems to be a problem here
		delete temp;
	}
}

(note the void return types) because you previously did not have them.... or are they not required in class embedded functions? i dunno because im new at this
also this is a linked list correct? shouldnt it be temp = temp->next;
?? or rather topofstack = temp?

constructors and destructors dont have return types

Seems like you're just deleting temp, or something.

I want to delete all of the nodes on the stack. Here's the header file I use with it:

Code:

typedef int StackType;

class Stack
{
public:
	Stack();
	~Stack();
	bool isEmpty();
	void push(StackType[2]);
	void pop(StackType[2]);


private:
	struct StackNode
	{
		StackType info[2];   //data item on the stack
		StackNode *next;   //a pointer to the next node
	};

	StackNode *topOfStack = new StackNode;  //a pointer to the first node in stack
};

When I debug the implementation file (in first post) it fails right where I noted.

Code:

while (topOfStack != NULL && topOfStack->next != NULL)
{
  temp = topOfStack;
  topOfStack = topOfStack->next; // Seems to be a problem here
  delete temp;
}

you forgot to check if topOfStack->next even points to anything
plus you'll want to initialize the topOfStack->next to either point to something, or else = NULL (0)
One other thing is it's probably better to put your initialization for topOfStack into your constructor; and also to check for errors, i.e.:

Code:

<snip>
	StackNode *topOfStack;  //a pointer to the first node in stack
};

Code:

Stack::Stack()
{
  try {
    topOfStack = new StackNode;
  } catch(...) {
    topOfStack = NULL;
  }
  if(topOfStack != NULL) topOfStack->next = NULL;
}

or use 0 instead of NULL -
same thing, although some say it's better to use NULL, and some say it's better to use 0... beats me but they both work.
Plus it's easier to say
if(topOfStack)
than it is to say
if(topOfStack != NULL)


Also, if all you do in your catch(...) statement is set topOfStack to NULL or 0, then later you will have to be sure to always check if it points to anything, or if it is NULL.

Cheers mate!

np - l8r m8