CA2000 when Returning Disposable Object from Method

Question

I have a factory method that builds objects that implement IDisposable. Ultimately it is the callers that manage the lifetime of the created objects. This design is triggering a bunch of CA2000 errors. Is there something fundamentally incorrect in my design, does it need refactoring, or is it just getting too excited about static code analysis warnings?

The factory method

public static DisposableType BuildTheDisposableType(string param1, int param2)
{
    var theDisposable = new DisposableType();

    // Do some work to setup theDisposable

    return theDisposable
}

A caller

using(var dt = FactoryClass.BuildTheDisposableType("data", 4))
{
   // use dt
}    

Answer 1

I would recommend that you suppress the CA2000 warning on each individual factory method, or perhaps on the entire class that contains them (but only if that is the only function of that class).

I further recommend that you include a justification:

[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Reliability",
    "CA2000:Dispose objects before losing scope",
    Justification = "This is a factory method. Caller must dispose")]

Answer 2

You're getting the error because the creator of the disposable object isn't managing it. However, there's nothing fundamentally wrong with the design. You are just relying on the consumers to leverage using. Not much different than the current ADO objects for example.

Answer 3

Another alternative is to change your factory method into a "configuration" method and to put the responsibility of creating the disposable object onto the client. Example:

public void SetupDisosableThing(IDisposable foo)
{
 foo.Bar = "baz";
}

void Main()
{
  using (var x = new Thing())
  {
   SetupDisposableThing(x);
  }
}

Answer 4

You should store it to local variable, and wrap initialization in the try-catch-rethrow block, dispose in case of any exception:

public MyDisposable CreateDisposable()
{
    var myDisposable = new MyDisposable();
    try
    {
        // Additional initialization here which may throw exceptions.
        ThrowException();
    }
    catch
    {
        // If an exception occurred, then this is the last chance to
        // dispose before the object goes out of scope.
        myDisposable.Dispose();
        throw;
    }
    return myDisposable;
}

Try to never leave the disposable object vulnerable to exceptions, when Dispose wouldn't be called

PS: someone previously mentioned to dispose inside the finally - this is obviously wrong - in non-exception path you don't want to call Dispose