Return to Answer

Do not use using namespace std.

In the constructor and push(), it looks like you're calling new on a function. It should be node.

Prefer nullptr to NULL and 0 for the node pointers if you're using C++11. Otherwise, you should choose either NULL or 0 for node pointers and keep it consistent.

You don't need this-> everywhere as the code already has the current object in scope.

It's needless to call pop() in the destructor. For one thing, pop() returns a value. You should just be using delete.

Instead, have head point to the first element, loop through the stack (stopping when NULL is hit) and call delete on each node. With this, you won't need to delete head at the end.

pop() and getTopElement() might be best as void. Here, a 0 is returned if the list is empty. Sure, no errors happening there. But then the calling code will take it as an element 0, never knowing whether or not the stack is empty. If you make them void, you can simply return if the stack is empty. As for retrieving an element, that's why you have getTopElement(). Once you get the value with getTopElement(), call pop() after that.

Regarding checking for an empty stack, you could make a public empty() function for calling code. The two functions above should still check for an empty stack, but containers like this usually utilizes such a function. This could go in the header:

 bool empty() const { return num_elements == 0; }

You could make your class more useful by using templates. This will allow you to populate the stack with any type instead of just ints.

Do not use using namespace std.

In the constructor and push(), it looks like you're calling new on a function. It should be node.

Prefer nullptr to NULL and 0 for the node pointers if you're using C++11. Otherwise, you should choose either NULL or 0 for node pointers and keep it consistent.

You don't need this-> everywhere as the code already has the current object in scope.

It's needless to call pop() in the destructor. For one thing, pop() returns a value. You should just be using delete.

Instead, have head point to the first element, loop through the stack (stopping when NULL is hit) and call delete on each node. With this, you won't need to delete head at the end.

pop() and getTopElement() might be best as void. Here, a 0 is returned if the list is empty. Sure, no errors happening there. But then the calling code will take it as an element 0, never knowing whether or not the stack is empty. If you make them void, you can simply return if the stack is empty. As for retrieving an element, that's why you have getTopElement(). Once you get the value with getTopElement(), call pop() after that.

Regarding checking for an empty stack, you could make a public empty() function for calling code. The two functions above should still check for an empty stack, but containers like this usually utilizes such a function. This could go in the header:

 bool empty() const { return num_elements == 0; }

You could make your class more useful by using templates. This will allow you to populate the stack with any type instead of just ints.

Do not use using namespace std.

In the constructor and push(), it looks like you're calling new on a function. It should be node.

It's best to have each head->next deal with NULL (or nullptr if using C++11) instead of 0. It's more accurate with memory addresses, and it'll help you differentiate them from actual values of 0 (such as for stack elements).

You don't need this-> everywhere as the code already has the current object in scope.

It's needless to call pop() in the destructor. For one thing, pop() returns a value. You should just be doing straight deletion. Instead, have head point to the first element, loop through the stack (stopping when NULL is hit) and call delete on each node. With this, you won't need to delete head at the end. But, to be on the safe side, you can set head to NULL instead.

pop() and getTopElement() might be best as void. Here, a 0 is returned if the list is empty. Sure, no errors happening there. But then the calling code will take it as an element 0, never knowing whether or not the stack is empty. If you make them void, you can simply return if the stack is empty. As for retrieving an element, that's why you have getTopElement(). Once you get the value with getTopElement(), call pop() after that.

Regarding checking for an empty stack, you could make a public empty() function for calling code. The two functions above should still check for an empty stack, but containers like this usually utilizes such a function. This could go in the header:

 bool empty() const { return num_elements == 0; }

For proper type use, size() and num_elements should be of type std::size_t .

You could make your class more useful by using templates. This will allow you to populate the stack with any type instead of just ints.

Do not use using namespace std.

In the constructor and push(), it looks like you're calling new on a function. It should be node.

Prefer nullptr to NULL and 0 for the node pointers if you're using C++11. Otherwise, you should choose either NULL or 0 for node pointers and keep it consistent.

You don't need this-> everywhere as the code already has the current object in scope.

It's needless to call pop() in the destructor. For one thing, pop() returns a value. You should just be using delete.

Instead, have head point to the first element, loop through the stack (stopping when NULL is hit) and call delete on each node. With this, you won't need to delete head at the end.

pop() and getTopElement() might be best as void. Here, a 0 is returned if the list is empty. Sure, no errors happening there. But then the calling code will take it as an element 0, never knowing whether or not the stack is empty. If you make them void, you can simply return if the stack is empty. As for retrieving an element, that's why you have getTopElement(). Once you get the value with getTopElement(), call pop() after that.

Regarding checking for an empty stack, you could make a public empty() function for calling code. The two functions above should still check for an empty stack, but containers like this usually utilizes such a function. This could go in the header:

 bool empty() const { return num_elements == 0; }

You could make your class more useful by using templates. This will allow you to populate the stack with any type instead of just ints.

Do not use using namespace std.

In the constructor and push(), it looks like you're calling new on a function. If node is your node type, then use that. It might be a good idea to post your header for more context.

It's best to have each head->next deal with NULL (or nullptr if using C++11) instead of 0. It's more accurate and safe with memory addresses, and it'll help you differentiate them from actual values of 0 (such as stack elements themselves).

You don't need this-> everywhere as the code already has the current object in scope.

It's needless to call pop() in the destructor. For one thing, pop() returns a value. You should just be doing straight deletion. Instead, have head point to the first element, loop through the stack (stopping when NULL is hit) and call delete on each node. With this, you won't need to delete head at the end. But, to be on the safe side, you can set head to NULL instead.

pop() and getTopElement() might be best as void. Here, a 0 is returned if the list is empty. Sure, no errors happening there. But then the calling code will take it as an element 0, never knowing whether or not the stack is empty. If you make them void, you can simply return if the stack is empty. As for retrieving an element, that's why you have getTopElement(). Once you get the value with getTopElement(), call pop() after that.

Regarding checking for an empty stack, you could make a public empty() function for calling code. The two functions above should still check for an empty stack, but containers like this usually utilizes such a function. This could go in the header:

 bool empty() const { return num_elements == 0; }

For proper type use, size()should return an std::size_t.

You could make your class more useful by using templates. This will allow you to populate the stack with any type instead of just ints.

Do not use using namespace std.

In the constructor and push(), it looks like you're calling new on a function. It should be node.

It's best to have each head->next deal with NULL (or nullptr if using C++11) instead of 0. It's more accurate with memory addresses, and it'll help you differentiate them from actual values of 0 (such as for stack elements).

You don't need this-> everywhere as the code already has the current object in scope.

It's needless to call pop() in the destructor. For one thing, pop() returns a value. You should just be doing straight deletion. Instead, have head point to the first element, loop through the stack (stopping when NULL is hit) and call delete on each node. With this, you won't need to delete head at the end. But, to be on the safe side, you can set head to NULL instead.

pop() and getTopElement() might be best as void. Here, a 0 is returned if the list is empty. Sure, no errors happening there. But then the calling code will take it as an element 0, never knowing whether or not the stack is empty. If you make them void, you can simply return if the stack is empty. As for retrieving an element, that's why you have getTopElement(). Once you get the value with getTopElement(), call pop() after that.

Regarding checking for an empty stack, you could make a public empty() function for calling code. The two functions above should still check for an empty stack, but containers like this usually utilizes such a function. This could go in the header:

 bool empty() const { return num_elements == 0; }

For proper type use, size() and num_elements should be of type std::size_t.

You could make your class more useful by using templates. This will allow you to populate the stack with any type instead of just ints.