a stack template problem

[ztdep]

Dear friends;
I write a stack template, the head file and the main are as follows; but when i test the push function, it give me the following error. i do not understand the reason. could you please help me out.

1>f:\computation\datastructure\stack template\stack template\main.cpp(31) : error C2664: 'Stack<T>::push' : cannot convert parameter 1 from 'double' to 'double &'

#include"StackTemplate.h"
#include<string>

int main()
	{
Stack<double> c;
c.push(1.5);
	  }

#ifndef STACK_TEMPLATE
#define STACK_TEMPLATE
#include <iostream>
#include<stdexcept>
using namespace std;

template<typename T> class Stack
	{
	private:
		 class Node
			 {
			 public:
				  T* pItem;  // Pointer to object T
				  Node* pNext; // Pointer to the next

				  // constructor a node from an object
				  Node(T& rItem):pItem(&rItem),pNext(0){}
				  Node():pItem(0),pNext(0){}
			  };
	     Node* pHead;
         void copy(const Stack& aStack);
		 void freeMemory();

	public:
		Stack():pHead(0){}     // default constructor for the stack

		Stack(const Stack& aStack);
		~Stack();

		Stack& operator=(const Stack& aStack);

		void push(T& rItem);
		T& pop();

		inline bool	  isEmpty(){return pHead==0;}
	};


template<typename T> void Stack<T>::push(T& rItem)
	{
	 	   Node* pNode=new Node(rItem);
		   pNode->pNext=pHead;
		   pHead=pNode;
	}


#endif

[mrnutty]

make it like so :

template<typename T> void Stack<T>::push(const T& val);

notice the constant.

[mike_2000_17]

-- Here is a description of what you should NOT do --

You could do what firstPerson suggests, but that would just create an error at line 41 saying that you cannot convert a const T& to a T& (usually, the message for that is: "error: converting parameter of type 'const T&' to 'T&' discards qualifiers").

Now, you could also make the constructor of Node to take a const T& as well, as so:

Node(const T& rItem):pItem(&rItem),pNext(0){}

But, then you will get yet another conversion error at that line because taking the address of a 'const T&' yields a pointer of type 'const T*', and thus, the statement pItem(&rItem) will again say: "error: converting pointer of type 'const T*' to a pointer of type 'T*' discards qualifiers".

Now, you could then make the pointer stored in Node, also be a const pointer, that is, you could write const T* pItem; .

That will get rid of your compilation errors. But it's the worst thing you could do!

-- Here is why the above is so bad --

There is a rule in C++ that allows you to take a temporary variable via a const reference. In other words, this is valid:

void Foo(const int& bar); //some function takes a const reference

int main() {
  Foo(42);     //the function can be called and the const ref can be binded to the temporary value 42, even though that "variable" has no "real" address in memory, at least, not a persistent or meaningful one.
};

This is obviously provided mainly for convenience (like having default values for passed-by-const-ref parameters, and other things like that). But it introduces a dangerous loop-hole. Since you can bind a temporary (i.e. called an rvalue) to a const-ref, and that you can take the address of a const-ref, this leads to the fact that you can, legally, take the address of a temporary variable, even though that address is not even really required to exist.

And, if you implement what I described above, by blindly solving the errors your compiler gives you, you will fall right into that loop-hole, and have a silent bug that could easily go unnoticed at first but give you terrible pain later. (A little lesson to learn here: don't blindly try anything to satisfy your compiler, understand why the error occurs and make sure you find a legal solution that is also good for your purpose, i.e. think before you act!).

-- Here is the little sticky note for your brain --

Always treat const-ref parameters as referring to a temporary variable, never take their address or keep the reference beyond the scope of the function, because you can't be sure if that address is valid, or for how long it will be (usually will become invalid as soon as the function returns).

-- Here are some solutions --

The "working but not so good" solution:
Pass-by-pointer all the way. Since you cannot take the address of a temporary directly, i.e. you cannot write const int* p = &42; , this will force the call-site to provide a non-temporary variable. Note that the caller can still provided a pointer to a very short-lived variable, so this solution is very brittle (and not very nice in terms of syntax).

The "vanilla" solution (as in the C++ STL):
Pass-by-const-ref and store a local copy. In your case, just make it so that Node stores the item by value, i.e., makes a local copy for itself. This is a very solid solution, because the node is in full ownership of the item it contains. However, this can be expensive if the contained item is large, and it can even be impossible if the contained item is not copyable (either on purpose or not).

The "shared-ownership" solution:
Pass-by-shared-pointer. Use the std::tr1::shared_ptr<T> class, this will allow you to share ownership between the caller and the callee. The item will be automatically deleted when both the caller and the callee are done with it (basic garbage collection via reference counting). This also avoids the copying and doesn't require the item to be copyable. However, of course, you pay some price both in performance and syntax-bloat, but, if needed, it is worth it, for robustness reasons.