Use of pointers in a stack class?

Question

I'm learning C++, and we were given an exercise to make a stack class using a class template and pointers. I'm not yet fully understanding the implementation of a stack or pointers so I gave it a go and made this class:

template <class T>
class Stack_Class {
public:
    T* stack;
    int item_quantity;
    T* First_item;
    int Max_quantity;

    Stack_Class(int value);
    ~Stack_Class();
    bool Add(T value);
    T Pop();
    int GetMax_Quantity();
    bool Full();
    bool Empty();
};


template <class T>
Stack_Class<T>::Stack_Class(int value) {
    if (value > 0) {
        stack = new T[value];
        First_item = stack;
        item_quantity = 0;
        Max_quantity = value;
    }
}

template <class T>
Stack_Class<T>::~Stack_Class() {
    if (First_item) {
        delete First_item;
    }
}



template<class T>
bool Stack_Class<T>::Add(T num) {
    if (item_quantity <Max_quantity) {

        *stack = num;
        stack++;
        item_quantity++;
        return true;
    }
    else return false;
}

template<class T>
T Stack_Class<T>::Pop() {
    if (!Empty()) {
        item_quantity--;
        return stack[item_quantity];
    }
     return NULL;
}

template<class T>
bool Stack_Class<T>::Empty() {
    return (item_quantity == 0);
}

template <class T>
int Stack_Class<T>::GetMax_Quantity() {
    return Max_quantity;
}

And the main class would be:

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

void main() {
    Stack_Class<int> intStack(3);

    intStack.Add(1);
    intStack.Add(2);
    intStack.Add(3);
    int count = intStack.GetMax_Quantity();

    for (int i = 0; i < count; i++) {
        std::cout << "Pop No: " << i << " - Elemento: "  << intStack.Pop() << std::endl;
    }
}

Though as a result I'm getting all random numbers instead of the ones I gave it in intStack. Add, so my question would be I'm implementing the pointer correctly here?

Answer 1

You are mixing up the pointer incrementing semantic in the Add method and the indexing semantic in the Pop method.

Since you need the index for the Empty method and so on, I would fix your Add method instead of the Pop as can be seen below.

Otherwise, you would end up still using the indexing in some method(s), but not in other. It would not look consistent to me.

template<class T>
bool Stack_Class<T>::Add(T num){
    if (item_quantity <Max_quantity){       

        stack[item_quantity++] = num;
        return true;
    }
    else return false;
}

Yet another problem in your code is this:

stack = new T[value];

but you seem to only delete the first element in the pointer. That is a guaranteed (and potentially not negligible) memory leak.

Even if you fix all that, your code would not still compile since you are trying to return void, whereas a C++ program should return int, so change this:

void main(){
    ...
}

to:

int main(){
    ...
}

... and return an integer like 0 correspondingly.

You would also need to fix this warning:

Stack_Class.h:56:13: warning: converting to non-pointer type ‘int’ from NULL [-Wconversion-null] return NULL; ^

By for instance change NULL to 0.

Having fixed all that, the output is like this:

Pop No: 0 - Elemento: 3
Pop No: 1 - Elemento: 2
Pop No: 2 - Elemento: 1

You can also see the code running on ideone.

For your convenience, this is the whole working code after those fixes:

template <class T>
class Stack_Class{
public:
    T* stack;
    int item_quantity;
    T* First_item;
    int Max_quantity;

    Stack_Class(int value); 
    ~Stack_Class();
    bool Add(T value);
    T Pop();
    int GetMax_Quantity();
    bool Full();
    bool Empty();   
};


template <class T>
Stack_Class<T>::Stack_Class(int value){
    if (value > 0){
        stack = new T[value];
        First_item = stack;
        item_quantity = 0;
        Max_quantity = value;
    }
}

template <class T>
Stack_Class<T>::~Stack_Class(){
    if (First_item){
        delete First_item;
    }
}



template<class T>
bool Stack_Class<T>::Add(T num){
    if (item_quantity <Max_quantity){       

        *stack = num;
        stack++;
        item_quantity++;
        return true;
    }
    else return false;
}

template<class T>
T Stack_Class<T>::Pop(){
    if (!Empty()){
        item_quantity--;
        return stack[item_quantity];
    }
     return NULL;
}

template<class T>
bool Stack_Class<T>::Empty(){
    return (item_quantity == 0);
}

template <class T>
int Stack_Class<T>::GetMax_Quantity(){
    return Max_quantity;
}

Answer 2

You need to deincrement the stack pointer before you reference it within Pop():

template<class T>
T Stack_Class<T>::Pop(){
    if (!Empty()){
        item_quantity--;
        stack--;
        return *stack;
    }
    return NULL;
}

Your array access stack[item_quantity] does not work because you increment stack in Add. So after construction, the memory pointed to by stack looks like this

0xff65f96f  <-- *(stack + 0)
0x0eec604f  <-- *(stack + 1)
0x05be0582  <-- *(stack + 2)
0x29b9186e  <-- *(stack + 3)

The hexdecimal values represent random garbage coincidentely located in the memory at the time of allocation. This is because memory allocated by new is not initialized to something nice. After adding three values, it looks like this

1           <-- *(stack - 3)
2           <-- *(stack - 2)
3           <-- *(stack - 1)
0x29b9186e  <-- *(stack + 0)
0xf66eff06  <-- *(stack + 1)
0x357eb508  <-- *(stack + 2)

In the first call of Pop, you access stack[2] = *(stack + 2), because item_quantity is 2 after deincrementing it. The two consecutive calls to Pop access stack[1] and stack[0]. As you can see above, you never actually reference the values you’ve put into the stack.