Thread: 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.

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.