I l@ve RuBoard | ![]() ![]() |
Solution![]() Count()The easiest of all Stack's members to implement safely is Count, because all it does is copy a builtin that can never throw. template<class T> size_t Stack<T>::Count() const { return vused_; // safe, builtins don't throw } No problem. Push()On the other hand, with Push, we need to apply our now-usual duty of care. template<class T> void Stack<T>::Push( const T& t ) { if( vused_ == vsize_ ) // grow if necessary { // by some grow factor size_t vsize_new = vsize_*2+1; T* v_new = NewCopy( v_, vsize_, vsize_new ); delete[] v_; // this can't throw v_ = v_new; // take ownership vsize_ = vsize_new; } v_[vused_] = t; ++vused_; } If we have no more space, we first pick a new size for the buffer and make a larger copy using NewCopy. Again, if NewCopy throws, then our own Stack's state is unchanged and the exception propagates through cleanly. Deleting the original buffer and taking ownership of the new one involves only operations that are known not to throw, so the entire if block is exception-safe. After any required grow operation, we attempt to copy the new value before incrementing our vused_ count. This way, if the assignment throws, the increment is not performed and our Stack's state is unchanged. If the assignment succeeds, the Stack's state is changed to recognize the presence of the new value, and all is well. Guideline
Pop() Goes the WeaselOnly one function left. That wasn't so hard, was it? Well, don't get too happy yet, because it turns out that Pop is the most problematic of these functions to write with complete exception safety. Our initial attempt might look something like this: // Hmmm... how safe is it really? template<class T> T Stack<T>::Pop() { if( vused_ == 0) { throw "pop from empty stack"; } else { T result = v_[vused_-1]; --vused_; return result; } } If the stack is empty, we throw an appropriate exception. Otherwise, we create a copy of the T object to be returned, update our state, and return the T object. If the initial copy from v_[vused_-1] fails, the exception is propagated and the state of the Stack is unchanged, which is what we want. If the initial copy succeeds, our state is updated and the Stack is in its new consistent state, which is also what we want. So this works, right? Well, kind of. There is a subtle flaw here that's completely outside the purview of Stack::Pop(). Consider the following client code: string s1(s.Pop()); string s2; s2 = s.Pop(); Note that above we talked about "the initial copy" (from v_[vused_-1]). That's because there is another copy[5] to worry about in either of the above cases—namely, the copy of the returned temporary into the destination. If that copy construction or copy assignment fails, then the Stack has completed its side effect (the top element has been popped off), but the popped value is now lost forever, because it never reached its destination (oops). This is bad news. In effect, it means that any version of Pop() that is written to return a temporary like this—and that therefore is responsible for two side effects—cannot be made completely exception-safe, because even though the function's implementation itself may look technically exception-safe, it forces clients of Stack to write exception-unsafe code. More generally, mutator functions should not return T objects by value. (See Item 19 for more about exception-safety issues for functions with multiple side effects.)
The bottom line—and it's significant—is this: Exception safety affects your class's design. In other words, you must design for exception safety from the outset, and exception safety is never "just an implementation detail." Common Mistake
The Real ProblemOne alternative—in fact, the minimum possible change[6]—is to respecify Pop as follows:
template<class T> void Stack<T>::Pop( T& result ) { if( vused_ == 0) { throw "pop from empty stack"; } else { result = v_[vused_-1]; --vused_; } } This ensures that the Stack's state is not changed unless the copy safely arrives in the caller's hands. But the real problem is that, as specified, Pop() has two responsibilities—namely, to pop the top-most element and to return the just-popped value. Guideline
So another option (and preferable, in my opinion) is to separate the functions of "querying the top-most value" and "popping the top-most value off the stack." We do this by having one function for each. template<class T> T& Stack<T>::Top() { if( vused_ == 0) { throw "empty stack"; } return v_[vused_-1]; } template<class T> void Stack<T>::Pop() { if( vused_ == 0) { throw "pop from empty stack"; } else { --vused_; } } Incidentally, have you ever grumbled at the way the standard library containers' pop functions (for example, list::pop_back, stack::pop, etc.) don't return the popped value? Well, here's one reason to do this: It avoids weakening exception safety. In fact, you've probably noticed that the above separated Top and Pop now match the signatures of the top and pop members of the standard library's stack<> adapter. That's no coincidence. We're actually only two public member functions away from the stack<> adapter's full public interface—namely: template<class T> const T& Stack<T>::Top() const { if( vused_ == 0) { throw "empty stack"; } else { return v_[vused_-1]; } } to provide Top for const Stack objects, and: template<class T> bool Stack<T>::Empty() const { return( vused_ == 0 ); } Of course, the standard stack<> is actually a container adapter that's implemented in terms of another container, but the public interface is the same and the rest is just an implementation detail. There's just one more fundamental point I want to drive home. I'll leave the following with you to ponder. Common Mistake
You will see Example 2 demonstrated very soon in this miniseries. Note that copy assignment operators may well elect to check for self-assignment even if they don't have to—for example, they might do so for efficiency. But a copy assignment operator that has to check for self-assignment (and else would not work correctly for self-assignment) is probably not strongly exception-safe. ![]() |
I l@ve RuBoard | ![]() ![]() |