CODEWITHSUNDEEP
c++stackruntime-error
Cereal
Cereal

Reputation: 39

Why does this code crash while testing stack?

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.

Upvotes: 0

Views: 132

Answers (4)

yosim
yosim

Reputation: 503

 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.

Upvotes: 1

Asinta
Asinta

Reputation: 31

The mistake is:

  1. stack s;//you define the local variable "s" without been intitialized.
  2. 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;
}

Upvotes: 0

Sarfaraz Nawaz
Sarfaraz Nawaz

Reputation: 361412

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.

Upvotes: 2

Vlad from Moscow
Vlad from Moscow

Reputation: 310980

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 };

Upvotes: 1

Related Questions