hello everybudy i'm writing a code to revers a stack but i have a wrong output when i Calling the reverse function can anyone help me please :) :)

header.h

class intnode
{
public :
    intnode(int , intnode*);
    int info;
    intnode* next ;

};
class Stack
{
public:
    Stack();
    void DisplayStack();
    int SizeOfStack();
    int Pop();
    void push(int);
    bool IsEmpty();
    void Reverc(Stack ,Stack,Stack);
    bool Plandrome();
private:
    intnode*top;
    int NumberOfNodes;

};

cpp source

#include<iostream>
#include"Header.h"
#include<process.h>
using namespace std;
intnode::intnode(int x , intnode*p=0)
{
    info = x ;
    next = p;
}
Stack::Stack()
{
    top=0;
    NumberOfNodes=0;
}
bool Stack::IsEmpty()
{
    return top==0;
}

int Stack::SizeOfStack()
{
    return NumberOfNodes;
}
void Stack::push(int x )
{

    if(IsEmpty())
    {
        top=new intnode(x);
        NumberOfNodes++;
    }
    else
    {
    top=new intnode(x,top);
    NumberOfNodes++;
    }
}
int Stack::Pop()
{

    if(IsEmpty())
    return NULL;
    else
    {
        int x=top->info;
        intnode*p=top;
        top=top->next;
        delete p;
        NumberOfNodes--;
        return x;
    }

}
void Stack::Reverc(Stack s,Stack d,Stack c)
{
    int x;  
    while(!s.IsEmpty())
    {
        x=s.Pop();
        d.push(x);

    }

    while(!d.IsEmpty())
    {
        x=d.Pop();
        c.push(x);
    }

    while(!c.IsEmpty())
    {
        x=c.Pop();
        s.push(x);
    }

}

void Stack::DisplayStack()
{
    intnode*p=top;
    cout<<"top -->"<<endl;

    while(p!=0)
    {
        cout<<"\t|\t"<<p->info<<"\t|"<<endl;
        p=p->next;
    }
    cout<<"\t  --------------"<<endl;

}
void main()
{
    Stack s ,d,c;
    s.push(5);
    s.push(7);
    s.Pop();
    s.push(77);
    s.push(44);
    s.Reverc(s,d,c);
    s.DisplayStack();
    cout<<"The Number Of Nodes In the stack is : "<<s.SizeOfStack()<<endl;
    system("pause");
}

The problem is that your reverse function takes its parameters by value, causing a (shallow) copy of the stacks:

void Stack::Reverc(Stack s,Stack d,Stack c)

This is wrong, for a few reasons. First of all, the stack that you seem to be reversing within that function is the s stack, but you really should be reversing the this stack, and this is the way that you call it in the main() function (i.e. you call reverse on a stack object and then print the result). In other words, with that correction, you get this:

void Stack::Reverc(Stack d, Stack c)
{
    int x;  
    while(!this->IsEmpty())
    {
        x = this->Pop();
        d.push(x);
    }
    while(!d.IsEmpty())
    {
        x=d.Pop();
        c.push(x);
    }
    while(!c.IsEmpty())
    {
        x = c.Pop();
        this->push(x);
    }
}

At this point, the result should be "correct", but there are still a couple of issues with the code. One issue, which is true in general, is that if you have variables like c and d that are only needed within the reversing function, then you should not pass them as parameters to the function, but instead, create them locally within the function:

void Stack::Reverc()
{
    Stack d, c;
    while(!this->IsEmpty())
        d.push( this->Pop() );
    while(!d.IsEmpty())
        c.push( d.Pop() );
    while(!c.IsEmpty())
        this->push( c.Pop() );
}

Another issue is that your code is leaking memory all over the place. The general rule in C++ is that every new allocation of memory must have a corresponding delete. You did this correctly in the push / pop functions by deleting nodes that are popped off the stack. However, you also need to worry about the destruction of the stack. Currently, when you destroy a stack object, all the memory it allocated (of remaining elements on the stack) will leak. So, you need a destructor for your stack class that will pop off all remaining elements (and thus, deleting them):

Stack::~Stack()
{
    while(!this->IsEmpty())
        this->Pop();
};

Another problem to fix is the issue that you have not defined a copy-constructor or assignment operator for your class. For example, if you make the mistake of passing a stack by value to a function (like you did already), you will have two stack objects that refer to the same "top" element (and all the elements below it), and when one stack pops it off (and deletes it) then the other stack will have a pointer to invalid memory. What you need to do in this case is to first create a deep copy-constructor:

Stack::Stack(const Stack& rhs) : top(0), NumberOfNodes(0)
{
    intnode* p_rhs = rhs.top;
    if( p_rhs == 0 )
        return;
    this->top = new intnode(p_rhs->info);
    NumberOfNodes = 1;
    intnode* p_this = this->top;
    p_rhs = p_rhs->next;
    while( p_rhs != 0 ) {
        p_this->next = new intnode(p_rhs->info);
        ++NumberOfNodes;
        p_this = p_this->next;
        p_rhs  = p_rhs->next;
    };
};

As you see, this constructor makes a deep copy of the "rhs" stack into the newly created one. This is important because the default behavior is to copy only the data members (copies the pointers, not the stuff they point to), which we call a shallow copy.

Then, you should create a swapping function to swap two stacks with each other. Note that swapping two stacks is simple, all you need to do is exchange the internal information, you don't need to do a complete element-by-element copy:

void Stack::Swap(Stack& rhs) 
{
    // swap the top pointer:
    intnode* p_tmp = rhs.top;
    rhs.top = this->top;
    this->top = p_tmp;
    // swap the number of nodes:
    int tmp_num = rhs.NumberOfNodes;
    rhs.NumberOfNodes = this->NumberOfNodes;
    this->NumberOfNodes = tmp_num;
};

I'll let you figure out how to implement a good assignment operator, and more importantly, how to re-implement your reversing function to make use of this Swap function.

For more details about this kind of stuff, check out this tutorial.

Comments
Very thorough!

I do not know how to thank you :)
you have given me new information I didn't know
and the program is running perfectly
thanks

This article has been dead for over six months. Start a new discussion instead.