patterncppModerate
C++ Stack using template
Viewed 0 times
templateusingstack
Problem
I'm learning C++, so I wrote a stack with the help of templates. I'm wondering if there is anything I can improve or if I did something vastly wrong.
Stack.h
Main.cpp
Stack.h
#pragma once
#include
template
struct Node {
Node(Type data, Node* next)
: data(data), next(next) {}
Node* next;
Type data;
};
template
class Stack
{
public:
Stack() : length(0), topNode(NULL) {
}
~Stack() {
while (!isEmpty()) {
pop();
}
}
void push(Type data) {
Node* newNode = new Node(data, topNode);
topNode = newNode;
++length;
}
Type pop() {
if (!isEmpty()) {
Node* popped = topNode;
Type poppedData = popped->data;
topNode = popped->next;
--length;
delete popped;
return poppedData;
}
throw new std::exception("Stack underflow!");
}
bool isEmpty() {
return length == 0;
}
void print() const {
Node* tempTop = topNode;
while (tempTop != NULL) {
std::cout data next;
}
}
int count() const {
return length;
}
private:
Node* topNode;
int length;
};Main.cpp
#include
#include "Stack.h"
using namespace std;
int main() {
Stack myStack;
// Push some values
myStack.push(2);
myStack.push(4);
myStack.push(8);
myStack.push(16);
myStack.push(32);
// We pop the 32
myStack.pop();
// Display count after the pop
int lastPopped = myStack.pop();
cout << "Popped value: " << lastPopped << ", Count: " << myStack.count() << endl;
// Print whole stack
cout << endl << "Stack print: " << endl;
myStack.print();
// Exit program when the 'any' key is pressed.
system("PAUSE");
return 0;
}Solution
If you provide a destructor you should handle copying and assigning. In other words follow the Rule of 3 (or 5 if you care about move semantics). You can also disallow them but then there should be move constructor and assignment.
Otherwise you will get in trouble with double freeing when calling a function
Consider adding a
The reason for using the reference is to avoid a unnecessary copy.
Having a print method interacting with the console is not a good idea. What if you want to show the contents in a gui instead?
Instead add iterators to be able to inspect what is on the stack without having to capture the output that
Otherwise you will get in trouble with double freeing when calling a function
void foo(Stack s).empty() doesn't change the stack. So it should also be const.Consider adding a
peek() method that returns a reference to the top value of the stack (with const and non-const version).T& peek()
{
return topNode->data;
}
const T& peek() const
{
return topNode->data;
}The reason for using the reference is to avoid a unnecessary copy.
Having a print method interacting with the console is not a good idea. What if you want to show the contents in a gui instead?
Instead add iterators to be able to inspect what is on the stack without having to capture the output that
print() generates.Code Snippets
T& peek()
{
return topNode->data;
}
const T& peek() const
{
return topNode->data;
}Context
StackExchange Code Review Q#102393, answer score: 11
Revisions (0)
No revisions yet.