Stack without use of STL - Version 1.1

Question

I had already posted this question at here and have tried to incorporate some of the suggestions given. Kindly let me know if there are still some concerns/suggestions/improvements in the code mentioned below.

#include"iostream"
#define MAX_SIZE 5
class Mystack
{
private:
 int A[MAX_SIZE];
 int top;
public:
 Mystack();
 void push(int x);
 int pop();
 int topElement();
 bool isEmpty();
 bool isFull();
 void print();
};
 Mystack::Mystack()
 {
 top = -1;
 }
 void Mystack::push(int x)
 {
 if(isFull())
 {
 std::cout << "Stack Overflow" << std::endl;
 return;
 }
 else
 {
 top++;
 A[top] = x;
 }
 }
 bool Mystack::isFull()
 {
 if (top == MAX_SIZE - 1)
 {
 std::cout << "Stack is full"<< std::endl;
 return true;
 }
 else
 {
 std::cout << "Stack is not full"<<std::endl;
 return false;
 }
 }
 int Mystack::pop()
 {
 if (isEmpty())
 {
 std::cout << "Stack Underflow" << std::endl;
 return 0;
 }
 else
 {
 std::cout << "The popped element is" << A[top];
 return A[top--];
 }
 }
 bool Mystack::isEmpty()
 {
 if (top == -1)
 {
 std::cout << "Stack is empty" << std::endl;
 return true;
 }
 else
 {
 std::cout << "Stack is not empty" << std::endl;
 return false;
 }
 }
 int Mystack::topElement()
 {
 std::cout<<"The top element is : "<< A[top];
 return A[top];
 }
 void Mystack::print()
 {
 for (int i = 0; i <=top; i++)
 {
 std::cout << A[i]<< " ";
 }
 }
void main()
{
 Mystack s1;
 int num,choice = 1;
 while (choice >0)
 {
 std::cout << "\n1. PUSH"<< std::endl;
 std::cout << "2. TOP" << std::endl;
 std::cout << "3. IsEmpty" << std::endl;
 std::cout << "4. POP" << std::endl;
 std::cout << "5. EXIT" << std::endl;
 std::cout << "6. Print" << std::endl;
 std::cout << "7. IsFull" << std::endl;
 std::cout << "Enter the choice" << std::endl;
 std::cin >> choice;
 switch (choice)
 {
 case 1:
 std::cout << "Enter the number to be pushed" << std::endl;
 std::cin >> num;
 s1.push(num);
 break;
 case 2:
 std::cout << "Get the TOP Element" << std::endl;
 s1.topElement();
 break;
 case 3:
 std::cout << "Check Empty" << std::endl;
 s1.isEmpty();
 break;
 case 4:
 std::cout << "POP the element" << std::endl;
 s1.pop();
 break;
 case 5: exit(0);
 case 6:
 s1.print();
 break;
 case 7: 
 s1.isFull();
 break;
 }
 }
 system("pause");
}

Answer 1

1. Don't mix a "library" class and input/output that is a bad style. If you really need to output some debug output, use a logger.
2. In push(), in case the a stack is full, you need to throw an exception or return an error code. Don't just silently ignore the error.
3. Same problem with the pop() method. Don't just ignore the error, notify the caller. And do not simply return 0, because it is a valid value that can be stored in a stack, so caller will not be able to distinguish error from successful call.

4. isFull() can be rewritten as a one-liner:

   isFull() { return top == MAX_SIZE - 1; }
   

5. Don't use #define to define a constant. Use const int. #define works as a simple text substitution and is a very old and very outdated way to define constants.

6. Add templates to your stack to support not only a stack of ints, but a stack of any type.
7. Is it a feature that your stack has a limited size? If yes, consider specifying max size through the constructor. If not, extend your internal array when the limit is reached.
8. The top variable is pointing to the last set element. Why not to change invariant to "point to the first free element" and replace this awkward top = -1 initialization with top = 0?
9. print() should not be in your stack class. Provide a generic way to iterate though your stack (read about how STL iterators are implemented or implement your own iterator) and let a user decide what to do with each value from a stack while iterating it.
10. You can replace top++; A[top] = x; with A[++top] = x. You are using similar style in other place of your class anyway.
11. Consider renaming array A. The name is not descriptive at all.
12. Well... "Mystack" name is awkward, rename it to "Stack" :)