I l@ve RuBoard Previous Section Next Section

Solution

graphics/bulb_icon.gif

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

graphics/guideline_icon.gif

Observe the canonical exception-safety rules: In each function, take all the code that might emit an exception and do all that work safely off to the side; only then, when you know that the real work has succeeded, should you modify the program state (and clean up) using only nonthrowing operations.


Pop() Goes the Weasel

Only 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梟amely, 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梐nd that therefore is responsible for two side effects梒annot 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.)

[5] For you experienced readers, yes, it's actually "zero or one copies," because the compiler is free to optimize away the second copy if the return value optimization applies. The point is that there can be a copy, so you have to be ready for it.

The bottom line梐nd it's significant梚s 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

graphics/commonmistake_icon.gif

Never make exception safety an afterthought. Exception safety affects a class's design. It is never "just an implementation detail."


The Real Problem

One alternative梚n fact, the minimum possible change[6]梚s to respecify Pop as follows:

[6] The minimum possible acceptable change, that is. You could always simply change the original version to return T& instead of T (this would be a reference to the popped T object, because for the time being the popped object happens to still physically exist in your internal representation), and then the caller could still write exception-safe code. But this business of returning references to "I no longer consider it there" resources is purely evil. If you change your implementation in the future, this may no longer be possible! Don't go there.



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梟amely, to pop the top-most element and to return the just-popped value.

Guideline

graphics/guideline_icon.gif

Prefer cohesion. Always endeavor to give each piece of code梕ach module, each class, each function梐 single, well-defined responsibility.


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梟amely:



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

graphics/commonmistake_icon.gif

"Exception-unsafe" and "poor design" go hand in hand. If a piece of code isn't exception-safe, that's generally okay and can simply be fixed. But if a piece of code cannot be made exception-safe because of its underlying design, that almost always is a signal of its poor design. Example 1: A function with two different responsibilities is difficult to make exception-safe. Example 2: A copy assignment operator that is written in such a way that it must check for self-assignment is probably not strongly exception-safe either


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梖or 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 Previous Section Next Section