Stack problem

[Sam99]

I have a simple problem. The code doesn't run correctly and crashing. The problem is in main(); in second loop. While it tries to pop out the infos, it crashes.
Can anyone figure it out and help me? Here is my code.
Thanks in advance..

#include <iostream>
#include <string>
#include <ctime>
using namespace std;

struct book_info{
	string ISBN;
	string title;
	string publisher;
	string author;
	int price;
};

struct movie{
	string name;
	string address;
	string phone_no;
	string gender;
	string ticket_type;
};

class stack{
private:
	book_info *st;
	int s;
	int top;
public:
	book_info input_info(book_info);

	stack(int);
	void push(book_info);
	void pop(book_info &);
};

book_info input_info(book_info book)
{
	cout<<"\nBook Info\n";
	cout<<"==============\n\n";

	cout<<"ISBN: ";
	getline(cin, book.ISBN);

	cout<<"Title: ";
	getline(cin, book.title);
		
	cout<<"publisher: ";
	getline(cin, book.publisher);

	cout<<"Author: ";
	getline(cin, book.author);

	cout<<"Price: ";
	cin>>book.price;
	
	return book;
}

stack::stack(int x)
{
	s=x;
	top=0;
	st=new book_info[s];
}

void stack::push(book_info x)
{
	if(top==s)
        cout<<"Stack full.\n";
	else{
		st[top]=x;
		++top;
	}
}

void stack::pop(book_info &x)
{
	if(top==0)
        cout<<"Stack empty.\n";
	else{
		st[top]=x;	
		--top;
		cout<<st[top].ISBN<<endl;
		cout<<st[top].title<<endl;
		cout<<st[top].publisher<<endl;
		cout<<st[top].author<<endl;
		cout<<st[top].price<<endl;
	}
}

int main()
{
	book_info book, n;
	int i, j;
	stack s1(2);	
	
	cout<<"Pushed values are:\n";
	for(i=0; i<2; ++i){
		
		book = input_info(n);
		cin.ignore();
		s1.push(book);
	}

	for(j=0; j<2; ++j){
		s1.pop(book);
	}
	
	cout<<endl;
	system ("Pause");
	return 0;
}

[long double main]

I ran your code through a debugger, and it's crashing on line 80:

void stack::pop(book_info &x)
{
    if(top==0)
        cout<<"Stack empty.\n";
    else{
        st[top]=x;	
        --top;

It would appear that st[top] is out-of-bounds.

My question is, why does your pop function accept a book_info argument, and what are you trying to do with it?
Since it's a stack, the pop function should just pop the top element off -- you shouldn't have to pass that element to the function.

[Sam99]

What I am trying: Consider some integer values instead of infos. Suppose, if I push values from 0 to 9, it will pop up the values 9 to 0. Just it.

I just wanna pop up the infos which are pushed into stack. :)

[long double main]

So then, what should happen if I were to write

stack myStack; // Assume integers instead of book_infos, for simplicity
for (int i = 0; i < 4; ++i)
    myStack.push(i);
// myStack now contains: 0 1 2 3 4

myStack.pop(2); // ??? 

?

pop should just remove the top element (and possibly return it, if you please). You shouldn't need to pass an argument for that.
Consider the pop member function of the standard std::stack container, for example:

void pop();

Remove top element
Removes the element on top of the stack, effectively reducing its size by one.

The element removed is the latest element inserted into the stack, whose value can be retrieved by calling member stack::top.

http://www.cplusplus.com/reference/stack/stack/pop/

Presumably, you should know which element is on top of your stack, so you don't need to be reminded by the user...right?

[Sam99]

Your pop() returns the top element of the stack. I understood this. :)

But, what if I push two values, and then wanna pop those from top to bottom? i.e., from last inserted element to first inserted element into the stack?

[Sam99]

Here is another code, which is little bit changed. I've used integers here, and the code works perfectly.

#include <iostream>
#include <ctime>
using namespace std;
class stack{
private:
	int *st;
	int s;
	int top;
public:
	stack(int);
	//void set_values();
	//void get_values();
	void push(int);
	void pop(int);
};

stack::stack(int x)
{
	s=x;
	top=0;
	st=new int[s];
}

/*
void stack::set_values()
{
	for(int i=0; i<s; ++i)
		st[i]=rand()%10;
}
void stack::get_values()
{
	for(int i=0; i<s; ++i)
		cout<<st[i]<<endl;
}
*/

void stack::push(int x)
{
	if(top==s)
        cout<<"Stack full.\n";
	else{
		st[top]=x;
		++top;
	}
}

void stack::pop(int x)
{
	if(top==0)
        cout<<"Stack empty.\n";
	else{
		st[top]=x;

		--top;
		cout<<st[top]<<endl;
	}
}

int main()
{
	int x, k, y;
	stack s1(10);
	//sc.set_values();
	//sc.get_values();

	cout<<"Pushed values are:\n";
	for(int i=0; i<10; ++i){
		cin>>k;
		s1.push(k);
	}

	cout<<"\nPoped values are:\n";
	for(int j=0; j<10; ++j)
		s1.pop(j);

	cout<<endl;
	return 0;
}

Can you please explain it now, why does this work, And why that doesn't? :)

[long double main]

pop should remove the top element from the stack.
When that happens, the second-to-top element becomes the new top element, so that will get popped if you call pop a second time.

[Sam99]

Thats the point I am talking about :).

so that will get popped if you call pop a second time.

It doesn't work for second time. I've tested with one (1) info, and it works. But when I push more than one info, it doesn't work out. :)

[long double main]

It works for me if I comment out line 80.

That aside, here's just a few other comments:
-- Line 80 (st[top]=x;) goes out of bounds when the stack is full. In fact, you don't need it since the only thing you really need to do to pop the stack is to decrement top (which you do correctly in the next line). So, you can just remove line 80. If you do that, then you also won't need your book_info &x parameter.

-- You declare input_info as a member function of stack on line 28, even though you don't define it as such (and it doesn't need to be a member function). I would just remove that line.

-- As far as the actual definition of input_info, you don't actually need to pass a parameter to it. If you just add book_info book; at the first line of the input_info function, it'll be fine. In that case, you also won't need your n variable in main().

-- I would actually move the cin.ignore(); from line 100 to after line 53. That way, you don't have to call it yourself every time.

[Sam99]

Its really a nice explanation. :)
Now I understand, I just need to decrement top as --.

Thanks a lot, sir! :)