Empty "using" statement in Dispose

Question

Recently I've seen some code written as follows:

public void Dipose()
{
   using(_myDisposableField) { }
}

This seems pretty strange to me, I'd prefer to see myDisposableField.Dispose();

What reasons are there for using "using" to dispose your objects over explicitly making the call?

Answer 1

No, none at all. It will just compile into an empty try/finally and end up calling Dispose.

Remove it. You'll make the code faster, more readable, and perhaps most importantly (as you continue reading below) more expressive in its intent.

Update: they were being slightly clever, equivalent code needs a null check and as per Jon Skeet's advice, also take a local copy if multi-threading is involved (in the same manner as the standard event invocation pattern to avoid a race between the null check and method call).

IDisposable tmp = _myDisposableField; 

if (tmp != null) 
    tmp.Dispose();

From what I can see in the IL of a sample app I've written, it looks like you also need to treat _myDisposableField as IDisposable directly. This will be important if any type implements the IDisposable interface explicitly and also provides a public void Dispose() method at the same time.

This code also doesn't attempt to replicate the try-finally that exists when using using, but it is sort of assumed that this is deemed unnecessary. As Michael Graczyk points out in the comments, however, the use of the finally offers protection against exceptions, in particular the ThreadAbortException (which could occur at any point). That said, the window for this to actually happen in is very small.

Although, I'd stake a fiver on the fact they did this not truly understanding what subtle "benefits" it gave them.

Answer 2

There is a very subtle but evil bug in the example you posted.

While it "compiles" down to:

try {}
finally
{
    if (_myDisposableField != null) 
        ((IDisposable)_myDisposableField).Dispose();
}

objects should be instantiated within the using clause and not outside:

You can instantiate the resource object and then pass the variable to the using statement, but this isn't a best practice. In this case, after control leaves the using block, the object remains in scope but probably has no access to its unmanaged resources. In other words, it's not fully initialized anymore. If you try to use the object outside the using block, you risk causing an exception to be thrown. For this reason, it's better to instantiate the object in the using statement and limit its scope to the using block.

—using statement (C# Reference)

In other words, it's dirty and hacky.

The clean version is extremely clearly spelled out on MSDN:

• if you can limit the use of an instance to a method, then use a using block with the constructor call on its border. Do not use Dispose directly.
• if you need (but really need) to keep an instance alive until the parent is disposed, then dispose explicitly using the Disposable pattern and nothing else. There are different ways of implementing a dispose cascade, however they need to be all done similarly to avoid very subtle and hard to catch bugs. There's a very good resource on MSDN in the Framework Design Guidelines.

Finally, please note the following you should only use the IDisposable pattern if you use unmanaged resources. Make sure it's really needed :-)

Answer 3

As already discussed in this answer, this is a cheeky way of avoiding a null test, but: there can be more to it than that. In modern C#, in many cases you could achieve similar with a null-conditional operator:

public void Dipose()
    => _myDisposableField?.Dispose();

However, it is not required that the type (of _myDisposableField) has Dispose() on the public API; it could be:

public class Foo : IDisposable {
    void IDisposable.Dispose() {...}
}

or even worse:

public class Bar : IDisposable {
    void IDisposable.Dispose() {...}
    public void Dispose() {...} // some completely different meaning! DO NOT DO THIS!
}

In the first case, Dispose() will fail to find the method, and in the second case, Dispose() will invoke the wrong method. In either of these cases, the using trick will work, as will a cast (although this will do slightly different things again if it is a value-type):

public void Dipose()
    => ((IDisposable)_myDisposableField)?.Dispose();

If you aren't sure whether the type is disposable (which happens in some polymorphism scenarios), you could also use either:

public void Dipose()
    => (_myDisposableField as IDisposable)?.Dispose();

or:

public void Dipose()
{
    using (_myDisposableField as IDisposable) {}
}