- 1.1k
- 3
- 14
- 27
That will not work for a T type that is not a pointer or integer. NULL can be assigned to integers because it is usually implemented as #define for 0. That's one of the reasons why you should use nullptr nullptr whenever possible (C++11). If you had used nullptr, your test with T=char would have failed and you would have noticed this problem.
Same goes for popping on an empty stack. Right now you are not generating any errors. You should consider throwing and exception. Deriving a StackUnderflow exception type from std::runtime_error std::runtime_error might be a good idea. Then you can extend the concept to a StackOverflow error when trying to push to the bounded stack. Granted that you can do with a simple std::runtime_error, but defining custom exception classes is a nice exercise if that's your point for writing this implementation.
Not a huge thing, but usually this kind of data structures uses std::size_t for things like size and capacity. That's an unsigned integer, which makes sense since the stack size is not meant to be negative, however, there is some discussion about avoiding unsigned integers avoiding unsigned integers, so that's up to you to decide if it is worth it.
That will not work for a T type that is not a pointer or integer. NULL can be assigned to integers because it is usually implemented as #define for 0. That's one of the reasons why you should use nullptr whenever possible (C++11). If you had used nullptr, your test with T=char would have failed and you would have noticed this problem.
Same goes for popping on an empty stack. Right now you are not generating any errors. You should consider throwing and exception. Deriving a StackUnderflow exception type from std::runtime_error might be a good idea. Then you can extend the concept to a StackOverflow error when trying to push to the bounded stack. Granted that you can do with a simple std::runtime_error, but defining custom exception classes is a nice exercise if that's your point for writing this implementation.
Not a huge thing, but usually this kind of data structures uses std::size_t for things like size and capacity. That's an unsigned integer, which makes sense since the stack size is not meant to be negative, however, there is some discussion about avoiding unsigned integers, so that's up to you to decide if it is worth it.
That will not work for a T type that is not a pointer or integer. NULL can be assigned to integers because it is usually implemented as #define for 0. That's one of the reasons why you should use nullptr whenever possible (C++11). If you had used nullptr, your test with T=char would have failed and you would have noticed this problem.
Same goes for popping on an empty stack. Right now you are not generating any errors. You should consider throwing and exception. Deriving a StackUnderflow exception type from std::runtime_error might be a good idea. Then you can extend the concept to a StackOverflow error when trying to push to the bounded stack. Granted that you can do with a simple std::runtime_error, but defining custom exception classes is a nice exercise if that's your point for writing this implementation.
Not a huge thing, but usually this kind of data structures uses std::size_t for things like size and capacity. That's an unsigned integer, which makes sense since the stack size is not meant to be negative, however, there is some discussion about avoiding unsigned integers, so that's up to you to decide if it is worth it.
Last thing is a bit of a big subject, which was introducesintroduced with C++11, called move semantics. Your code does some unnecessary copying of data on push and StackNode's constructor, which might affect performance when T is something other that a native type or pointer. I'll leave you with a couple references you can read to further learn about this:
Last thing is a bit of a big subject, which was introduces with C++11, called move semantics. Your code does some unnecessary copying of data on push and StackNode's constructor, which might affect performance when T is something other that a native type or pointer. I'll leave you with a couple references you can read to further learn about this:
Last thing is a bit of a big subject, which was introduced with C++11, called move semantics. Your code does some unnecessary copying of data on push and StackNode's constructor, which might affect performance when T is something other that a native type or pointer. I'll leave you with a couple references you can read to further learn about this: