C# abstract Dispose method

abstract classcdisposestatic analysis

I have an abstract class that implements IDisposable, like so:

public abstract class ConnectionAccessor : IDisposable
{
    public abstract void Dispose();
}

In Visual Studio 2008 Team System, I ran Code Analysis on my project and one of the warnings that came up was the following:

Microsoft.Design : Modify 'ConnectionAccessor.Dispose()' so that it calls Dispose(true), then calls GC.SuppressFinalize on the current object instance ('this' or 'Me' in Visual Basic), and then returns.

Is it just being silly, telling me to modify the body of an abstract method, or should I do something further in any derived instance of Dispose?

Best Answer

You should follow the conventional pattern for implementing Dispose. Making Dispose() virtual is considered bad practice, because the conventional pattern emphasizes reuse of code in "managed cleanup" (API client calling Dispose() directly or via using) and "unmanaged cleanup" (GC calling finalizer). To remind, the pattern is this:

public class Base
{
    ~Base()
    {
        Dispose(false);
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this); // so that Dispose(false) isn't called later
    }

    protected virtual void Dispose(bool disposing)
    {
        if (disposing)
        {
             // Dispose all owned managed objects
        }

        // Release unmanaged resources
    }
}

Key here is that there's no duplication between finalizer and Dispose for unmanaged cleanup, and yet any derived class can extend both managed and unmanaged cleanup.

For your case, what you should do is this:

protected abstract void Dispose(bool disposing)

and leave everything else as is. Even that is of dubious value, since you're enforcing your derived classes to implement Dispose now - and how do you know that all of them need it? If your base class has nothing to dispose, but most derived classes likely do (with a few exceptions, perhaps), then just provide an empty implementation. It is what System.IO.Stream (itself abstract) does, so there is precedent.

Related Topic