Which is the most correct pattern for using Stack.Pop?

Question

There at least 2 patterns of using Stack.Pop

1. private Stack<T> availableResources;
   ...
   if (availableResources.Count > 0) 
   {
     resource = availableResources.Pop();
     ...
   }
   

2. private Stack<T> availableResources;
   ...
   try 
   {
     resource = availableResources.Pop();
     ...
   }
   catch (InvalidOperationException e)
   {
     ...
   }
   

I'd like to ask people, which of 2 is preferable? If possible, argument your answer.

Answer 1

Program defensively and use option 1.

Since the Stack class provides the ability to check whether it contains anything before trying to Pop a value from it, you should do the check and avoid using the exception for flow control.

If you want to avoid doing the if check throughout your application, you could create a TryPop extension.

public static class StackExtensions
{
    public static bool TryPop<T>(this Stack<T> stack, out T value)
    {
        if (stack.Count > 0)
        {
            value = stack.Pop();
            return true;
        }

        value = default(T);
        return false;
    }
}

It could be used in the following way:

T resource;
if (availableResources.TryPop(out resource)) 
{
    // use `resource` for something
}

It's worth noting that this approach is only suitable if the Stack is not accessed concurrently! - If it is, and you are using .NET 4.0 onwards then you should use ConcurrentStack, otherwise create your own concurrent stack which encapsulates a Stack and manages access to it with locks.

Answer 2

First we should check, is there concurrent access to stack, or not. If it is, the first pattern doesn't handle an exception ('cause between the check and popping itself there can be an additional popping from the other thread), so consider using either ConcurrentStack<> or double-checking lock. Without them, in concurrent case, the second pattern is definitely better.