Search code examples
c#code-analysis

Using block galore?


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?


Solution

  • 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.