Reputation: 65
I'm trying to create a stack using linked lists in c++. But the display function i have written prints only the top of the stack. I can't really understand why this is happening. Any help or clarification is much appreciated. Thanks
#include<iostream.h>
#include<conio.h>
class Node
{
protected:
Node* next;
int data;
public:
Node(int d){data=d;}
friend class Stack;
};
class Stack
{
public:
Stack(){top->next='\0';length=0;}
void push(int d)
{
Node *n=new Node(top->data);
n->next='\0';
top->next=n;
top->data=d;
length++;
}
int pop()
{
top=top->next;
length--;
return top->data;
}
void displaystack()
{
while(top->next!='\0')
{
cout<<top->data<<endl;
top=top->next;
}
}
int getlength()
{
return length;
}
private:
Node *top;
int length;
};
void main()
{
clrscr();
Stack s;
s.push(9);
s.push(8);
s.push(7);
s.push(6);
s.push(5);
s.push(3);
s.displaystack();
int len=s.getlength();
cout<<"length of stack is "<<len<<endl;
getch();
}
It only prints the following: 3 the length of stack is 6
--------xxxxxxx-------xxxxxxxx--------xxxxxxx-----------xxxxxxxxxxxxx--------------
After editing the code looks like this: and works too! (thanks to @Kaathe) :P
#include<iostream.h>
#include<conio.h>
class Node
{
protected:
Node* next;
int data;
public:
Node(int d){data=d;}
friend class Stack;
};
class Stack
{
public:
Stack(){top->next=NULL;length=0;}
~Stack()
{
while(top!=NULL)
{
Node* toDelete=top;
top=top->next;
delete toDelete;
}
}
void push(int d)
{
Node *n=new Node(d);
n->next=top;
top=n;
length++;
}
int pop()
{
Node* oldtop=top;
top=top->next;
int oldtopdata=oldtop->data;
delete(oldtop);
--length;
return oldtopdata;
}
void displaystack()
{
Node* current=top;
while(current->next!=NULL)
{
cout<<current->data<<endl;
current=current->next;
}
}
int getlength()
{
return length;
}
private:
Node *top;
int length;
};
Upvotes: 0
Views: 24892
Reputation: 1
You can replace while(top->next!='\0')
to while(top!='\0')
It should work then...
Upvotes: 0
Reputation: 672
I would be surprised if your program works if you still have this in your code:
Stack(){top->next=NULL;length=0;}
Solution is left as exercise for the reader ;-)
Upvotes: 0
Reputation: 335
When you push
, you want to create an entirely new Node
, set its data to the value d
, and point it at the old top of the stack. You can then set the top of the stack to this new node. You actually don't need to modify the old top node at all.
So push could read:
void push(int d)
{
Node* newTop = new Node(d);
newTop->next = top;
top = newTop;
++length;
}
I just noticed that pop
also won't behave as expected, as you throw away the top node, and return the data in the node below it. Maybe you could write:
int pop()
{
Node* oldTop = top;
top = top->next;
int oldTopData = oldTop->data;
delete(oldTop);
--length;
return oldTopData;
}
It would probably also be a good idea to stop people from popping empty stacks (for example by checking that length >= 1
at the start of pop()
.
Finally, displaystack
will kind of destroy the Stack
object if called, by losing the pointer to the top node. Maybe this would be better:
void displayStack()
{
Node* currNode = top;
while(currNode->next != nullptr)
{
std::cout << currNode->data << std::endl;
currNode = currNode->next;
}
}
It makes more sense to me to end the linked list with a nullptr
too.
Also, the stack should have a destructor which delete
s all its Node
s - I'll let you write that one ;)
Upvotes: 2
Reputation: 2013
When you print (in displaystack
) you should use a temporary variable instead of destructively updating the top variable.
To avoid a memory leak in pop()
you should also delete
the node previously allocated using new
.
Upvotes: 0