You have two problems.
Problem 1 is the one you have run in to is the *head in *head = input; dereferences the pointer. *head = input; says "Get me the value head points at and assign input to this value." head doesn't point anywhere usable, so the program fails. What you ALMOST want is head = &input;, point head at input.
But here comes problem 2! input has been automatically allocated and, like any automatic variable, will go out of scope at the end of its code block, the end of the function in this case, and be destroyed and freed, leaving you with a pointer to an invalid object. Much badness will result at some point in the future when the program tries to use this invalid object.
"Wait a second," you say. "I dynamically allocated that sucker with
StackItem *temp = new StackItem(s);
Yes. Yes you did. But similar to above the *temp in
stack->push(*temp);
means "Get me the value temp points at." This value is then passed by value, and copied, into push. This copy goes out of scope at the end of the function. The original pointed at by temp is leaked when temp goes out of scope at the end of the function. Without a pointer to a dynamic allocation, it is next to impossible to find and free the allocation later, and if you don't free unused memory, it sits around taking up space until (assuming the program is running on a modern operating system) the OS cleans up all of the program's memory when the program exits.
One possible solution is
void Stack::push(StackItem * input) // pass in a pointer to a dynamically allocated StackItem
// ^ passing by pointer
{
input.next = head; // just do it. It's always going to be safe, or you have a bigger bug
// coming, and the test to see if you NEED to do it may cost more than
// doing it.
head = input;
}
and is called something like
StackItem *temp = new StackItem(s);
stack->push(temp);
// ^ no dereference
Before you read any further, remember: Do what the teacher asks and pass the course. You may have to learn to program for real later, but you have a piece of paper that says you know how to program . Society puts a lot of weight on that piece of paper. And maybe the teacher is setting up mistakes so that they can be covered in detail later or allowing them to happen to keep the amount of information they have to shovel into your brain all at once as light as possible. Hard to say.
My personal preference looks more like this:
void Stack::push(const std::string & input) // reference to string that will not be modified.
{
head = new StackItem{ input, head }; // aggregate initialization
}
and usage:
stack.push(s);
To make this work,
stack is not dynamically allocated. Rather than
stack = new Stack(); // Whoops! Leaked any previous Stack!
You use
Stack stack;
In C++ you should use new as infrequently as possible so you don't have as much of a mess to clean up. Everything you new, you need to delete or you have leaks. In addition to not freeing memory, another common bug is freeing memory too soon. It is hard to manually manage memory, even for the experts. So do what the experts do: Not do it. They use automatic allocations or automatic allocations of a class that manages/owns a dynamic allocation (or other non-memory resource like a file or a mutex). See Resource Allocation Is Initialization (RAII) for more information.
StackItem becomes a private nested struct hidden inside Stack so no one outside knows it even exists. A struct is just like a class in every way you're ever likely to notice except classes default to private access and struct defaults to public. Since no one can see a private nested class except the class that contains it, encapsulation is maintained so long as the nested class is just a simple grouping of data and requires no special defenses from misuse by the class it is nested in. When a class is this simple, you generally don't need a constructor and can take advantage of simple Aggregate Initialization.
class Stack
{
private:
struct StackItem
{
std::string value
StackItem * next;
}
// rest of Stack
};
In a good Object Oriented system, the user knows nothing of what's happening inside the object. They don't know there's a linked list inside. All they know is the call push on it with a string and the string goes onto the stack. This way you can perform a wholesale rewrite of the internals of Stack, maybe base it on a dynamic array, and no one outside will ever know. They just push strings and when they pop, they get strings.
As soon as you expose an inner implementation detail like StackItem, the user knows more about your class, and the more they know, the more resistant to change they become. If you rewrite Stack such that StackItem changes, users of Stack must also change. Better that they not be exposed to StackItem in the first place.
