Return to Answer

You never actually call a Node using the default values, so you can simplify the constructor to:

Node(T t, Node* link) :item{t}, next{link} { }

It also only copies the parameters, so they can be written as const references

Node(const T &t, const Node* &link) :item{t}, next{link} { }

As Abrixas2 said, you did not delete the now unused pointer in pop

All this requires is a temporary variable to hold the location of first

template<class T>
T Stack<T>::pop() {
 if (empty()) {
 throw std::out_of_range("underflow");
 }
 auto t = first -> item;
 auto temp = first;
 first = temp -> next;
 delete temp;
 --n;
 return t;
}

Now you should see some things start to fail.

When you made a copy, because there was no specific copy constructor, you only did a shallow copy, getting a copy of the pointer first (which will link to the same node as x) and the current size n (which makes it now completely independent of x). When you popped the values, you changed the pointer first only in y, but the values in x still point to the undeleted data (with the change above, this should point at garbage). Again, n was only copied once, so variable x did not get informed about anything getting deleted or size changing because it was only seen in y

You never actually call a Node using the default values, so you can simplify the constructor to:

Node(T t, Node* link) :item{t}, next{link} { }

It also only copies the parameters, so they can be written as const references

Node(const T &t, const Node* &link) :item{t}, next{link} { }

As Abrixas2 said, you did not delete the now unused pointer in pop

All this requires is a temporary variable to hold the location of first

template<class T>
T Stack<T>::pop() {
 if (empty()) {
 throw std::out_of_range("underflow");
 }
 auto t = first -> item;
 auto temp = first;
 first = temp -> next;
 delete temp;
 --n;
 return t;
}

Now you should see some things start to fail.

When you made a copy, because there was no specific copy constructor, you only did a shallow copy, getting a copy of the pointer first (which will link to the same node as x) and the current size n (which makes it now completely independent of x). When you popped the values, you changed the pointer first only in y, but the values in x still point to the undeleted data (with the change above, this should point at garbage). Again, n was only copied once, so variable x did not get informed about anything getting deleted or size changing because it was only seen in y

You never actually call a Node using the default values, so you can simplify the constructor to:

Node(T t, Node* link) :item{t}, next{link} { }

It also only copies the parameters, so they can be written as const references

Node(const T &t, const Node* &link) :item{t}, next{link} { }

As Abrixas2 said, you did not delete the now unused pointer in pop

All this requires is a temporary variable to hold the location of first

template<class T>
T Stack<T>::pop() {
 if (empty()) {
 throw std::out_of_range("underflow");
 }
 auto t = first -> item;
 auto temp = first;
 first = temp -> next;
 delete temp;
 --n;
 return t;
}

Now you should see some things start to fail.

When you made a copy, because there was no specific copy constructor, you only did a shallow copy, getting a copy of the pointer to first and the current size of x. When you popped the values, you changed the pointer to first only in y, but the values in x still point to the undeleted data. Again, count was only copied once, so variable x did not get informed about anything getting deleted or size changing because it was only seen in y

You never actually call a Node using the default values, so you can simplify the constructor to:

Node(T t, Node* link) :item{t}, next{link} { }

It also only copies the parameters, so they can be written as const references

Node(const T &t, const Node* &link) :item{t}, next{link} { }

As Abrixas2 said, you did not delete the now unused pointer in pop

All this requires is a temporary variable to hold the location of first

template<class T>
T Stack<T>::pop() {
 if (empty()) {
 throw std::out_of_range("underflow");
 }
 auto t = first -> item;
 auto temp = first;
 first = temp -> next;
 delete temp;
 --n;
 return t;
}

Now you should see some things start to fail.

When you made a copy, because there was no specific copy constructor, you only did a shallow copy, getting a copy of the pointer first (which will link to the same node as x) and the current sizen (which makes it now completely independent of x). When you popped the values, you changed the pointer first only in y, but the values in x still point to the undeleted data (with the change above, this should point at garbage). Again, n was only copied once, so variable x did not get informed about anything getting deleted or size changing because it was only seen in y

You never actually call a Node using the default values, so you can simplify the constructor to:

Node(T t, Node* link) :item{t}, next{link} { }

It also only copies the parameters, so they can be written as const references

Node(const T &t, const Node* &link) :item{t}, next{link} { }

As Abrixas2 said, you did not delete the now unused pointer in pop

All this requires is a temporary variable to hold the location of first

template<class T>
T Stack<T>::pop() {
 if (empty()) {
 throw std::out_of_range("underflow");
 }
 auto t = first -> item;
 auto temp = first;
 first = temp -> next;
 delete temp;
 --n;
 return t;
}

You never actually call a Node using the default values, so you can simplify the constructor to:

Node(T t, Node* link) :item{t}, next{link} { }

It also only copies the parameters, so they can be written as const references

Node(const T &t, const Node* &link) :item{t}, next{link} { }

As Abrixas2 said, you did not delete the now unused pointer in pop

All this requires is a temporary variable to hold the location of first

template<class T>
T Stack<T>::pop() {
 if (empty()) {
 throw std::out_of_range("underflow");
 }
 auto t = first -> item;
 auto temp = first;
 first = temp -> next;
 delete temp;
 --n;
 return t;
}

Now you should see some things start to fail.

When you made a copy, because there was no specific copy constructor, you only did a shallow copy, getting a copy of the pointer to first and the current size of x. When you popped the values, you changed the pointer to first only in y, but the values in x still point to the undeleted data. Again, count was only copied once, so variable x did not get informed about anything getting deleted or size changing because it was only seen in y