Using block galore?

Question

I'd like your opinion on the following subject:

Imagine we have a method that is responsible of achieving one specific purpose, but to do so, it needs the support of an important number of locally scoped objects, many of them implementing IDisposable.

MS coding standards say that when using local IDisposable objects that need not "survive" the scope of the method (will not be returned or will not be assigned to the state information of some longer lived object for instance) you should use the using construct.

The problem is, that in some situations you can get a nested "hell" of using blocks:

using (var disposableA = new DisposableObjectA())
{
     using (var disposableB = new DisposableObjectB())
     {
          using (var disposableC = new DisposableObjectC())
          {
               //And so on, you get the idea.
          }
     }
}

You can somehow mitigate this if some of the objects you are using derive from a common base or implement a common interface that implements IDisposable. This of course comes at a cost of having to cast said objects whenever you need the true type of the object. Sometimes this can be viable as long as the amount of casting doesn't get out of hand:

using (var disposableA = new DisposableObjectA())
{
     using (DisposableBaseObject disposableB = new DisposableObjectB(),
            disposableC = new DisposableObjectC)
     {
          using (var disposableD = new DisposableObjectD())
          {
               //And so on, you get the idea.
          }
     }
}

The other option is not to use using blocks and implement directly try-catch blocks. This would look like:

DisposableObjectA disposableA = null;
DisposableObjectB disposableB = null;
DisposableObjectC disposableC = null;
...

try
{
    disposableA = new DisposableObjectA();
    ....
}
finally
{
     if (disposableA != null)
     {
          disposableA.Dispose();
     }

     if (disposableB != null)
     {
          disposableB.Dispose();
     }

     //and so on
}

Funny thing, is that VS Code Analyzer will flag this code as "wrong". It will inform you that not all possible execution paths ensure that all disposable objects will be disposed before going out of scope. I can only see that happening if some object throws while disposing which in my opinion should never happen, and if it does, its normally a sign that something really messed up is going on and you are probably better of exiting as fast and gracefully as you can from your entire app.

So, the question is: what approach do you like better? Is it always preferable to use nested using blocks no matter how many, or, past a certain limit, its better to use try-catch block?

Answer 1

You don't need the curly brackets if there's just one statement, for example:

using (var disposableA = new DisposableObjectA())
using (var disposableB = new DisposableObjectB())
using (var disposableC = new DisposableObjectC())
{
               //And so on, you get the idea.
}

This does depend on nothing else happening in the outer blocks though.

Answer 2

I think you are forgetting that the using statement (like many others), don't necessarily require a code block, but can be a single statement. Your first example can be written as:

using (var disposableA = new DisposableObjectA())
using (var disposableB = new DisposableObjectB())
using (var disposableC = new DisposableObjectC())
{
    //And so on, you get the idea.
}

I think this eases the problem tremendously. Note, it doesn't help if you need to do something between the invocation of the instances that implement IDisposable.

I even go so far as to nest other blocks that make sense. foreach is an example.

IEnumerable<int> ints = ...;

using (var disposableA = new DisposableObjectA())
using (var disposableB = new DisposableObjectB())
using (var disposableC = new DisposableObjectC())
foreach (int i in ints)
{
    // Work with disposableA, disposableB, disposableC, and i.
}

It should be noted that VS Code analyzer is correct when it tells you that this is incorrect:

DisposableObjectA disposableA = null;
DisposableObjectB disposableB = null;
DisposableObjectC disposableC = null;
...

try
{
    disposableA = new DisposableObjectA();
    ....
}
finally
{
     if (disposableA != null)
     {
          disposableA.Dispose();
     }

     if (disposableB != null)
     {
          disposableB.Dispose();
     }

     //and so on
}

When you use using stacked on top of each other, it nests them in multiple try/finally blocks, like so:

DisposableObjectA disposableA = null;
DisposableObjectB disposableB = null;
DisposableObjectC disposableC = null;
...

try
{
    disposableA = new DisposableObjectA();

    try
    {
        disposableB = new DisposableObjectB();

        // Try/catch block with disposableC goes here.
    }
    finally
    {
         if (disposableB != null)
         {
              disposableB.Dispose();
         }    
    }
}
finally
{
     if (disposableA != null)
     {
          disposableA.Dispose();
     }    
}

In your example, if there is an exception that is thrown when disposableA.Dispose is executed, then disposableB and disposableC do not get disposed (the finally block is exited), if an error is thrown when disposableB is called, then disposableC is not closed, etc.

Answer 3

The only real 'problem' with your first code sample is the deep nesting, which can make reading and maintaining code hard. As an alternative to the other answers suggesting you simply drop the curly braces, you can also work round this by refactoring the most deeply nested code out into a separate function. This separates the concerns of creating and disposing your disposable objects from actually using them.

using (var a = new DisposableObjectA())
{
    using (var b = new DisposableObjectB())
    {
         using (var c = new DisposableObjectC())
         {
              SomeFunction(a,b,c);
         }
    }
}