C++ Stack using template

by

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

#pragma once
#include <ostream>

template <class Type>
struct Node {
    Node(Type data, Node<Type>* next) 
        : data(data), next(next) {}
    Node* next;
    Type data;
};

template <class Type>
class Stack
{
public:

    Stack() : length(0), topNode(NULL) {
    }

    ~Stack() {
        while (!isEmpty()) {
            pop();
        }
    }

    void push(Type data) {
        Node<Type>* newNode = new Node<Type>(data, topNode);
        topNode = newNode;
        ++length;
    }

    Type pop() {
        if (!isEmpty()) {
            Node<Type>* 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<Type>* tempTop = topNode;
        while (tempTop != NULL) {
            std::cout << tempTop->data << endl;
            tempTop = tempTop->next;
        }
    }

    int count() const {
        return length;
    }

private:
    Node<Type>*         topNode;
    int                 length;

};

Main.cpp

#include <iostream>
#include "Stack.h"

using namespace std;

int main() {
    Stack<int> 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;
}

Answer

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 void foo(Stack<int> 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.

Attribution
Source : Link , Question Author : Caresi , Answer Author : ratchet freak

Leave a Comment