I wrote a piece of code, can't find where is the bug....

Dear All:

Here is the code I wrote.

Code:

#include <iostream>
using namespace std;

template <typename T>
class Stack;

template <typename T>
class Node{
private:
	Node * next;
	T* data;
public:
	friend class Stack<T>;
	Node():next(0),data(0){}
	Node( Node * head,  T * value):next(head),data(value){}
	Node * GetNext(){ return next; }
	void SetNext(const Node* newnext) {next = newnext;}
	T* GetData() { return data; }
	void SetData(const T* newdata) { data = newdata;}
	virtual ~Node(){}
};

template <typename T>
class Stack{
public:
	Stack():head(0){}
	void Push( T* data);
	T * Pop();
	T * Top();
	virtual ~Stack();
private:
	Node<T> * head;
};

template <typename T>
void Stack<T>::Push( T* data){
	head = new Node<T>(head,data);
}

template <typename T>
T* Stack<T>::Pop(){
	if (!head) return NULL;
	Node<T> * oldhead = head;
	T * result = head->GetData();
	head = head->GetNext();
	delete(oldhead);
	return result;
}

template <typename T>
T* Stack<T>::Top(){
	if (!head) return NULL;
	return (head->GetData());
}

template <typename T>
Stack<T>::~Stack(){
	while(head){ 
		Node<T>* oldhead = head;
		head = head->GetNext();
		delete(oldhead);
	}
}



int main(){
	Stack<int>* test = new Stack<int>;
	int x=1,y=2,z=3;
	test->Push(&x);
	test->Push(&y);
	test->Push(&z);
	cout<<*(test->Top())<<endl;
	test->Pop();
	cout<<*(test->Top())<<endl;
	delete(test);

	return 0;
}

This code implements a Stack.
I tried to declare: (with const)

Code:

Node( const Node * head,  const T * value):next(head),data(value){}

But it fails to complie... I am so confused....

BTW, I would polish this Stack as safe as possible, any suggestions to improve the robustness?

delete is only used for variables that have been dynamically allocated with new. You don't use it on ordinary variables. The destructor will be called anyway for the variable when it goes out of scope, when main (and thus your program) exits.

Thank you, dwk.
How about the question about 'const'?

>> But it fails to complie...
What was the error message?

>> I would polish this Stack as safe as possible, any suggestions to improve the robustness?
Don't store pointers, store objects.

Originally Posted by Daved

>> But it fails to complie...
What was the error message?

error C2440: 'initializing' : cannot convert from 'const Node<T> *' to 'Node <T> *'

Originally Posted by Daved

>>

>> I would polish this Stack as safe as possible, any suggestions to improve the robustness?
Don't store pointers, store objects.

I don't think store objects is a good idea.
For example, if the object is an instance of

Code:

class Ugly{
char tmp[10];
};


Ugly node;
Stack<Ugly> test;
test.Push(node);

Then the Node's copy constructor doesn't really "copy" the array but the pointer to the array.
Unfortunately we can't design the Node<T> better becasue Node<T> knows nothing of the data structure of T.

No, but then a programmer does this:

Code:

class Nice
{
  // includes various data types,
  // proper constructors, copy constructors, assignment, etc.
  // overall, a well built-class.
};

void somefunction(Stack<Nice> &test) {
...
  Nice node;
  // setting of node's members, etc.
  test.Push(&node);
} // node goes out of scope, and test now contains an pointer to doom.

This same design issue occurs with many of the std:: containers - and they store objects. You're right that the Node can't copy - this job is the classes, and if a class has data that requires a special copy constructor/assignment operator, then they should provide code for it, or use a Stack<Ugly *> to avoid it, and handle the pitfalls themselves.

As Cactus_Hugger said, its the responsibility of the class to copy itself properly and the responsibility of the user of the stack to decide whether to store objects or pointers. By writing the stack to store object, you give the user a choice, since that "object" might be a pointer (e.g. Stack<Ugly *>).

As for your compiler error, I guess the problem is that you have a pointer to a const node, and then you try to assign it to the member variable which is a pointer to a non-const node. I believe that since the const version promises to not change the object, but the non-const version doesn't, the compiler won't let you make the assignment. I don't think there is anything wrong with leaving it without the const, though.