Presumably, "underflow" is an instance of some exception class. Standing on its own, what you've posted is not valid code. Probably your professor was just trying to give an example -- you shouldn't expect examples to necessarily work or even compile, they are just demonstrating a concept.

I would agree, on the basis that popping an empty stack is an undefined operation.

Says who?

Whether throwing an exception is the right thing or not, there's no reason not to specify a particular behavior for an invalid use case. Libraries which blow into a billion pieces when misused are bad libraries. The only reason the C standard leaves so many things undefined is because that's the only way to get people to agree to the standard, and to get implementation feasibility across a broad range of architectures. For a special purpose library it's rather bad form IMHO to just detonate when someone calls the library improperly.

I don't think you're meant to surround every pop() with a try-catch to detect failure. Several thoughts:

1. In most usages of a stack, the push and pop of an element matches up lexically in the code, making it a logical impossibility for it to ever pop an empty stack
2. Even if you don't know how many items are on the stack you can just query to see if it's empty or not

In other words, properly working code would never try to pop an item from an empty stack. But it does no harm to throw an exception if it DOES happen. Getting crashes down inside library code due to your OWN bugs is infuriating. Is it a bug in your code, or in the library? If the library throws a YoureStupidException when you mess up it makes it rather clear who's problem it is

I would agree, on the basis that popping an empty stack is an undefined operation.

O_o

And how would you express "popping an empty stack is an undefined operation" with code that doesn't involve intentionally writing code that is undefined?

Also, exceptions (IMO) should be used to express _exceptional_ conditions that cannot be recovered from at the point they are discovered.

At the point where the client has already invoked the given operation the method can't recover from the condition; the method can report the error in multiple ways, ignore it in multiple ways, or do something insane like pushing a default object only to then pop it, but it can not magic away the nature of "client wants to remove something that doesn't exist".

What would you like to do?

Ignore the condition and perform the underlying operation just the same? (Which could do anything depending on the platform.)

Ignore the condition and simply do nothing? (Implying that trying to remove something from something that is itself empty is perfectly reasonable.)

Return a boolean that notes whether or not the function succeeded? (Which implies that the operation can fail in any event meaning code littered with the relevant checks.)

Kill the application? (Which suggests that you don't know what you are doing.)

Well its obviously a matter of opinion in this case. For example, STL stack<T>:op() does not throw, and neither does vector<T>:perator[], either of which could throw an exception but instead do some implementation-defined behavior or simply remain undefined.

Return a boolean that notes whether or not the function succeeded? (Which implies that the operation can fail in any event meaning code littered with the relevant checks.)

Actually, for any given call to pop(), it could fail. So I think this is the best solution. But there all the points made above are valid.

For example, STL stack<T>p() does not throw, and neither does vector<T>erator[], either of which could throw an exception but instead do some implementation-defined behavior or simply remain undefined.

What `std::stack<???>:op()' does when `std::stack<???>::empty()' returns true is implementation defined.

Actually, for any given call to pop(), it could fail.

Actually, given the requirements on contained types required by the standard it is possible for a stack implementation to satisfy the traditional interface requirements and never fail when the stack isn't empty because the underlying tools must never fail.

So let's say we're using a stack that fluctuates, as in infix to postfix expression conversions. At any given moment the stack may be empty, and if so it would abort because of the underthrow? What would be a good alternative?

So let's say we're using a stack that fluctuates, as in infix to postfix expression conversions. At any given moment the stack may be empty, and if so it would abort because of the underthrow? What would be a good alternative?

In that case, the only reason the stack becomes empty before being popped is due to a syntax error in the input, which IMHO warrants an exception.

Allow me to preface my statements with I feel that all of the approaches discussed here are valid and have merit. Ultimately I think it comes down to design choice and perhaps the general error handling design of the larger system to maintain consistency across it.

Having said that here is my opinion and my design choice. In my opinion the stack merely provides functionality for popping, pushing, and is guaranteed not to explode b/c of user error. If you try to pop an empty stack it should do absolutely nothing since the stack is empty. I see no reason to throw an exception since there is no harm done if the stack is smart enough to figure it out. I believe the object can be coded in such a way as to prevent this type of behavior from ever happening instead of forcing someone to wrap try/catch around every pop operation but that is probably another approach altogether.

Return a boolean that notes whether or not the function succeeded? (Which implies that the operation can fail in any event meaning code littered with the relevant checks.)

If you throw you then force the client to wrap every pop in a try / catch which is far uglier than a simple if. Regardless of whether you return an error or throw you force extra checks so your statement about a boolean is the exact same statement and produces the exact same type of error handling code as a throw.

In that case, the only reason the stack becomes empty before being popped is due to a syntax error in the input, which IMHO warrants an exception.

It is not a library error, though. It is a user error and one that the library implementor decides how to handle. It is impossible to catch all user errors and I would argue that one should not try b/c no matter how many user errors or babysitters you code into your system you end up getting smarter idiots that still crash the system. However, one could argue that blowing up would show the problem immediately but I would argue should a stack who knows about all things stack really try to pop an empty stack? I am a human that knows about gravity and if someone tells me to continue walking towards a 100 ft ledge I'm going to stop at some point, ignore the request, and not walk off of it. If I try to shift my car from 5th to 1st at 80 mph my transmission literally refuses to do it due to the design of the transmission, synchros, etc. It doesn't shut the car off or cut the fuel as that would be dangerous....but it does not perform the requested gear shift.

The stack in my opinion will know if the stack is empty or not and therefore can simply opt to do nothing if a pop is requested on an empty stack. Is it really a critical error that some moron on the outside requested a pop b/c he/she cannot properly use the stack? A pop is requested, the stack determines it is an incorrect request and does nothing. No harm no foul. Sure the client code could be littered with tons of pop statements when they are doing nothing but I do not think it is the responsibility of the stack class to ensure that client code doesn't call pop 50 times when the stack is empty. If you put an empty() function in the stack interface then there is enough information in the interface for clients of the stack class to use it properly. As well this could be documented in the design docs for the stack which would explain the behavior.

If you throw and it is not caught and handled it will bring the program down. If you return a bool and it is not checked, the program continues on. If you do nothing and do not perform the operation, the program continues on. If you want to continue on as if nothing happened then the last two are your only choice. Alternatively you could handle the throw internally but then you are throwing to tell yourself that something bad happened and that is just ridiculous.

On another note you could approach this from the requirements and usage of the system. If this is going to run someone's pacemaker then it would be unethical and near criminal not to notify that something bad happened. If it is running in something less critical then perhaps you might do something different. The STL examples are valid but the STL was designed so that all C++ programmers past and present can use the thing with little effort and little trouble. Your stack may not have those same requirements and use cases. In the end it is a pure judgement call as to what you do.

It is impossible to catch all user errors and I would argue that one should not try b/c no matter how many user errors or babysitters you code into your system you end up getting smarter idiots that still crash the system.

I have to address this because it is so very relevant to your framing of this issue, but before I say anything let me quote something else relevant to this quote.

If you try to pop an empty stack it should do absolutely nothing since the stack is empty.

And with that one more.

Is it really a critical error that some moron on the outside requested a pop b/c he/she cannot properly use the stack?

Okay. Now with those quotes in mind lets take a minute to discuss why you are arguing mutually exclusive ideals.

If you are not going to "babysit" the client code you can't wrap every routine with precondition guards that would allow for the operation to "do nothing" in the face of popping from an empty stack. "Babysitting", "Precondition Guards", "Exception Mechanisms" or "Simple Design" it doesn't really matter what you call it; the nature of the operation can't work with an empty stack so you either check for an empty stack or very likely crash the application. Expecting the application to crash in the face of violating a condition is fine, but that's not what you are advocating.

If you are going to "babysit" the client code you can safely catch the inconstancy of "remove something that doesn't exit" you would, for the sake of consistency, be expected to code similar checks into every function. By definition you are advocating "babysitting" because you are suggesting that such operations, which for example includes "access array element that doesn't exist", should not yield errors or unexpected behavior despite of their obvious error nature.

Sure the client code could be littered with tons of pop statements when they are doing nothing but I do not think it is the responsibility of the stack class to ensure that client code doesn't call pop 50 times when the stack is empty.

O_o

But that responsibility is exactly what you've placed on the library code. If you think the way the error is caught matters, I'm afraid to say that you are wrong. The method of handling the error is irrelevant. It doesn't matter if you check for an empty stack and "do nothing", raise an exception, or return a boolean; all of those options, every one, is putting the responsibility of protecting against misuse on the library code.

If you put an empty() function in the stack interface then there is enough information in the interface for clients of the stack class to use it properly.

This is true regardless of the method used to handle the error. If my implementation throws an exception in the considered case the user would have the option to check the preconditions themselves before attempting to call the method.

As well this could be documented in the design docs for the stack which would explain the behavior.

Yes. As could this:

Code:

Use:
The `void Stack<???>::pop()' method removes the "topmost" element, the last element placed onto the stack with the most recent call to the `void Stack<???>::push(ElemT &)' method.
Precondition:
The `bool Stack<???>::isEmpty()' method must return `false'.
Notes:
In debug mode, `NDEBUG' is not defined, the `void Stack<???>::pop()' method will raise an exception upon precondition violation, otherwise violation of the precondition is undefined.

If you throw you then force the client to wrap every pop in a try / catch which is far uglier than a simple if.

O_o

Well, I wasn't going to respond to this, but let me go ahead and smack that notion out of you.

Such a notion is simply false; I don't know where you got the notion, but you need to relieve yourself of that ignorance as soon as is possible.

You never need to use `try' and `catch' blocks unless you are going to do something with the exception.

Looking at the considered case, let us compare the three options.

If client code knows the precondition is valid, no option requires special handling.

Assuming a "do nothing" approach if client code doesn't know if the operation is valid and successful continuation requires a successful `pop' the client code must literally guess.

Assuming a "return a boolean value" approach if client code doesn't know if the operation is valid and successful continuation requires a successful `pop' the client code must check every attempt.

Assuming a "raise an exception" approach if client code doesn't know if the operation is valid and successful continuation requires a successful `pop' the client may assume that the operation was successful for if not the exception mechanism would have triggered and processing would not have continued.