Why does this code crash while testing stack?

Question

OK, so I edited my code, but I still have two problems :

But here's my code first :

    #include <iostream>
using namespace std;

struct stack
{
    int data[5];
    int top;
};

void push (int a, stack &S)
{
    S.top++;

    if (S.top<5)
    {
        S.data[S.top]=a;
    }
    else cout<<"Stack is full!!!"<<endl; S.top--;
}


int pop(stack &S)
{
    if (S.top==-1)
    {
        cout<<"Stack is empty!"<<endl;
    }
    else
    {
        int temp=S.data[S.top];
        S.data[S.top]=NULL;
        S.top--;
        return temp;
    }
}

bool isEMPTY(stack &S)
{
    if (S.top==-1)
        return true;
    else return false;
}
bool isFULL(stack &S)
{
    if (S.top==5)
        return true;
    else return false;
}


int main()
{
    stack S = { {}, -1 };

    push(5,S); cout<<"5 is pushed \n"<<endl;
    push(3,S); cout<<"3 is pushed \n"<<endl;
    push(1,S); cout<<"1 is pushed \n"<<endl;
    push(2,S); cout<<"2 is pushed \n"<<endl;
    push(6,S); cout<<"6 is pushed \n"<<endl;
    push(7,S); cout<<"7 is pushed \n"<<endl;

    cout<<pop(S)<<"is popped\n"<<endl;
    cout<<pop(S)<<"is popped\n"<<endl;
    cout<<pop(S)<<"is popped\n"<<endl;

    return 0;
}

So, the first problem is, when I pop I get a "Totally random value" and it's not like LIFO.

Second is, I actually intended on inserting 6 values, when I already had the max value = 5, so the output actually showed me the 6 values.

Look at the value of `s.top` when you're debugging. – Mat Dec 01 '13 at 13:10 — Mat, Dec 01 '13 at 13:10

Nawaz · Answer 1 · 2013-12-01T13:23:34.877

2

stack S;

Since stack is POD, the above line doesn't initialize the member top. As such, using an uninitialized top in push and pop functions invokes undefined behavior.

Write this:

stack S {}; //must be compiled in C++11 mode, else write : stack S = stack();

This value-initializes S and its members, which means, top is initialized to 0. The rest of the code may still have other problems, but at least you have fixed the issues with proper initialization. If you work with 0 as initial value of top, you've write the logic of push and pop accordingly!

Once you fix that, check the value of top before pushing and poping values from the stack, as the member array can have at most 5 elements, and you cannot pop more elements when it is empty. You must maintain these invariants.

edited Dec 01 '13 at 13:23

answered Dec 01 '13 at 13:11

Nawaz

353,942
115
666
851

When I tried to do this, it gave me a redline under the first " { " saying : expected a ";" . – Cereal Dec 01 '13 at 13:19
And how to check the value of top in main ? – Cereal Dec 01 '13 at 13:21
It is invalid definition if an object of type stack because according to the logic top should be initialized as -1. I showed the correct definition in my post.:) – Vlad from Moscow Dec 01 '13 at 13:21
@user3054349: You should check `top` in `push()` and `pop()` functions, not outside them. – Nawaz Dec 01 '13 at 13:22
I already did so, in the function itself for push() and pop() each one alone, I had a condition for the function. – Cereal Dec 01 '13 at 13:23
And when I try to pop, it gives me some value as : 948394829 is popped. – Cereal Dec 01 '13 at 13:24
@user3054349: I'll not explain that part. Debug your code. Use `cout` to print the value of `top` every time you `push` and `pop` an element, see if its value makes sense. See when things dont make sense. Try to run the code in your brain. You have to THINK A LOT to make your code perfect. – Nawaz Dec 01 '13 at 13:26
Ok, so I ran the code 'In my Brain', and it makes SENSE. But when I tried to trace the value of top, I found it doesn't change, so it's like I didn't even do anything, and the top doesn't store. So the error here is in the code, not in my logic. I'm new to programming, please help. – Cereal Dec 01 '13 at 13:31
Use `cout` in `push` and `pop` to print `top` and other values. I can fix the issue, but that would be useless for you. I can tell you how you can debug your code *yourself* without *requiring* anyone fixing the issue for you. – Nawaz Dec 01 '13 at 13:35
Sir, I'm confused about this whole thing, it's that I already have issues with the basics, so please at least let me know how can I fix the syntax. – Cereal Dec 01 '13 at 13:42
@user3054349: You need a BOOK to know the basics and the syntax. It is very frustrating to teach the *basics* online in such forum. If your code doesn't compile, search for the error, and know what is wrong with your code. – Nawaz Dec 01 '13 at 13:45
Can you tell me what is the name of the book that can help me ?! – Cereal Dec 01 '13 at 14:35
@user3054349: Buy any introductory book from this list : [The Definitive C++ Book Guide and List](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list)... I would suggest **C++ Primer** by ***Stanley Lippman, Josée Lajoie, and Barbara E. Moo***. – Nawaz Dec 01 '13 at 14:38

Vlad from Moscow · Answer 2 · 2013-12-01T13:18:45.503

1

I do not see where an object of type stack was created and how data member top was initialized. Also take nto account that member function push does not check whether there is an attempt to add an item beyond the array.

You should define the object the following way

stack S = { {}, -1 };

edited Dec 01 '13 at 13:18

answered Dec 01 '13 at 13:12

Vlad from Moscow

301,070
26
186
335

score 1 · Answer 3 · answered Dec 01 '13 at 13:40

 else cout<<"Stack is full!!!"<<endl; S.top--;

is identical to :

else
{
cout<<"Stack is full!!!"<<endl;
} 
S.top--;

as a general rule, try to avoid: writing if/else without curly brackets, and, avoid writing more then one line of code in the same line.

score 0 · Answer 4 · answered Dec 01 '13 at 13:37

The mistake is:

stack s;//you define the local variable "s" without been intitialized.
push(5,s);//pass the uninlitialized "s" to the function "push",when debugging your code,"s.top" is not a expected "-1",but some value incredible~(often extreamly larger than 5),so your push operation failed!

stack S;
S.top = -1;
for(int i = 0; i < 5; i++)
{
    S.data[i] = 0;
}

Why does this code crash while testing stack?

4 Answers4