Pop function returning stack elments in reverse order

[oceanSpray]

Hi ya'll, I'm creating a story using three stacks which will hold strings for nouns, verbs, and adjectives. I created a pop method which returns the string popped off the stack. I then created another method to display the story, however whenever I display the story, the items are popped off in First in, first out order instead of Last in First out order.

How the stack prints:
Ex: noun stack

spy
package
mr. smith
dead drop

pop Function:

string StackStory::popStory()
{
   StackNode *temp = nullptr;
   string str ="";
   
   //Make sure stack is empty
   if(isEmpty())
   {
       cout<<"Stack is empty\n";
   }
   else     //pop value off stack 
   {
       str = top->value;
       temp = top->next;
       delete top;
       top = temp;
       
       
   }
   return str;
}

Make story function:

void makeStory(StackStory &noun, StackStory &verb, StackStory &adj)
{
    string noun_item = "";
    string verb_item = "";
    string adj_item = "";
    
    
    cout<< "The " << noun.popStory() <<" was cautious about where to " << verb.popStory()  << " the " << noun.popStory() << " containing "<<
        adj.popStory()<< " infromation about "<< noun.popStory()<<".\n If the "<<noun.popStory()<< " were to be compromised, it could be the start of a "<<
            adj.popStory()<< " situation.\n\n";
    
    
}

makeStory prints

The dead drop.... was cautious...
//instead of
The spy.. was cautious...

[jonnin]

let us see 'push' ? pop looks ok.
and dumb question check... what did you type in first, second...

[Ganado]

Note that the default constructor for std::string already is an empty string, so you don't need to do = "". Just string str; is fine.
Also, temp can be declared within the else statement because it's only used locally inside that branch.

jonnin is correct, there's not enough information here to see what is going wrong.

[oceanSpray]

jonnin, here's my push:

void StackStory::push(string str)
{
    const int MAX_WORD_LENGTH=9;
    
    //Pointer to a node 
    StackNode *newNode = nullptr;
    
    //Creating new node
    newNode = new StackNode;
    //Storing string value in str
    newNode->value = str;
    
    if(str == "" || str.length() > MAX_WORD_LENGTH)
    {
        cout<<"Word Requirements:\n"<<
            "1. Must not leave entry blank\n"<<
            "2. Word should be no longer than 10 letters\n";
            exit(EXIT_FAILURE);
    }
    //If there are no nodes in list
    // make newNode the first node
    else if(isEmpty())
    {
        //top will point at newNode
        top = newNode;
        //newNode linker set to null
        newNode->next = nullptr;
    }
    else    //Insert newNode before top LIFO
    {
        //newNode linker will point to what is currently at the top of stack
        newNode->next = top;
        //newNode will now become the top of the stack
        top = newNode;
        
        cout<<"Value popped onto list.\n";
    }
    
    
}

If you're asking what order I added the strings to my stack first, it would be "dead drop", "Mr.Smith", "package", "spy".

I am also calling the makeStory function in the main, passing it 3 stack objects

[jonnin]

I don't see anything that would do what you said happened here.
note that if you can safely assume that top == null when its empty, then the second else is sufficient without the first isempty block.

both push and pop seem fine...

[Ganado]

I re-implemented your code, removed the comments and extraneous letter limit, and added a small driver.

It works fine for me. The problem is something else. Try debugging and stepping through your code with break statements, looking at the variables, and seeing when something doesn't match your expectations. Visual Studio has a great debugger (I'm sure there are pros at gdb as well, but I have not been able to learn it very well yet).

#include <iostream>
#include <string>

using namespace std;

struct StackNode {
	string value;
	StackNode* next;
};

struct Stack {
	StackNode* top;
	
	string pop();
	void push(string str);
	bool isEmpty();
};

string Stack::pop()
{
   string str;

   if(isEmpty())
   {
       cout<<"Stack is empty\n";
   }
   else
   {
       str = top->value;
       StackNode *temp = top->next;
       delete top;
       top = temp;           
   }
   return str;
}

bool Stack::isEmpty()
{
	return (top == nullptr);
}

void Stack::push(string str)
{
    StackNode *newNode = new StackNode;
    newNode->value = str;
    
    if(isEmpty())
    {
        top = newNode;
        newNode->next = nullptr;
    }
    else 
    {
        newNode->next = top;
        top = newNode;
    } 
}

int main()
{
	Stack stack;
	
	stack.push("wookie");
	stack.push("nookie");
	stack.push("cookie");
	
	cout << stack.pop() << '\n';
	cout << stack.pop() << '\n';
	cout << stack.pop() << '\n';
}

cookie
nookie
wookie

(My Stack is not a complete example; it of course has a memory leak if all nodes are not manually popped. It is just an example to show that the excerpts that oceanSpray has shown are OK.)