Creating a stack class in C++

Question

Create a class called Stack for storing integers. The data members are an integer array for storing the integers and an integer for storing the top of stack (tos). Include member functions for initializing tos to 0, pushing an element to the stack and for popping an element from the stack. The push() function should check for “stack overflow” and pop() should check for “stack underflow”.

#include<iostream>
using namespace std;
#define SIZE 50
class Stack {
    int stackArray[SIZE];
    int tos;
public:
    Stack() {
        tos = 0;
    }
    void push(int);
    int pop();
    void set(int);


};

void Stack::push(int value) {

    if (tos < SIZE) {
        stackArray[tos++] = value;
    }
    else
        cout << "Stack overflow" << endl;

}
void Stack::set(int a) {
    tos = a;

}
int Stack::pop() {
    if (tos == 0) {
        cout << "Stack underflow " << endl;
    }
    else {
        return stackArray[--tos];
    }

}
int main() {
    Stack s;
    for (int i = 0; i < 50; i++) {
        s.push(i);
    }
    for (int i = 0; i < 51; i++) {
        cout << s.pop() << " " << i << endl;
    }
}

Answer 1

Overall, this looks pretty good. If it were me, I'd use a more descriptive name for the top of the stack such as topOfStack or just top. tos doesn't really mean anything, so without a comment describing it, I would be confused seeing that in code. (And if you need to write a comment to explain a variable name, you've probably named it incorrectly.)

I'm not sure your set() method works correctly. From the description it sounds like it shouldn't take any arguments and should set tos to 0. But it's possible I misunderstood. If you set the top to some passed-in value, then you have a bunch of junk on the stack between its previous value and the current value, which probably isn't what you want.

Answer 2

Prefer constants to macros

Preprocessor macros are tricky to use correctly - they have no namespace, no type and global scope. We can replace SIZE with a private constant member:

#include <cstdint>

class Stack
{
     // private
     constexpr std::size_t array_length = 50;
}

I renamed it, because we might later want to creat size() and capacity() members to act like the standard containers.

Avoid using namespace std;

The std namespace isn't one that's designed to be imported wholesale like that. Avoid surprises, and be clear where identifiers come from, by importing just the names you need, in the smallest reasonable scope - or just qualify them fully (std:: is intentionally very short!).

Don't mix errors with output

Error messages should be streamed to std::cerr rather than std::cout.

Improve the error reporting

Do you see the problem with pop()?

int Stack::pop() {
    if (tos == 0) {
        cout << "Stack underflow " << endl;
    }
    else {
        return stackArray[--tos];
    }
}

In the underflow case, there's no return statement. Silly errors like that suggest that you compiled without a good set of warnings enabled.

We don't have any way to indicate to the program (rather than the end user) that overflow or underflow has occurred; the C++ way to do so is to throw an exception in those cases. Then the program can decide whether and how to inform the user about the problem.