DisposableObject base class for C#

Question

I commonly run into the need to implement IDisposable in my code. To correctly dispose of both managed and unmanaged resources requires a reasonable amount of boilerplate code. Based on the documentation found here I have created the following base class.

My intent is that any object I create that needs to dispose of resources doesn't need to rewrite the boilerplate code. Rather it can inherit from this object and implement two abstract methods.

Does this appear correct? And has anyone else written something similar? I would appreciate comments on the style, correctness and how well this conforms to best practices.

public abstract class DisposableObject : IDisposable
{
    private bool _disposed = false;

    public bool Disposed
    {
        get { return _disposed; }
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    ~DisposableObject()
    {
        Dispose(false);
    }

    private void Dispose(bool disposing)
    {
        if (!_disposed)
        {
            if (disposing)
            {
                DisposeManagedResources();
            }

            DisposeUnmanagedResources();

            _disposed = true;
        }
    }

    protected abstract void DisposeManagedResources();
    protected abstract void DisposeUnmanagedResources();
}

EDIT: Final Implementation

public abstract class DisposableObject : IDisposable
{
    public bool Disposed { get; private set;}      

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    ~DisposableObject()
    {
        Debug.Assert(Disposed, "WARNING: Object finalized without being disposed!");
        Dispose(false);
    }

    private void Dispose(bool disposing)
    {
        if (!Disposed)
        {
            if (disposing)
            {
                DisposeManagedResources();
            }

            DisposeUnmanagedResources();
            Disposed = true;
        }
    }

    protected virtual void DisposeManagedResources() { }
    protected virtual void DisposeUnmanagedResources() { }
}

Answer 1

I would think on making two changes:

1. Make one or both Dispose*Resources methods virtual instead of abstract. Though it highly depends on how often do you need to handle unmanaged resources. I can't remember last time I was handling them and I would hate overriding this method in each class just to make it empty.
2. I would add some logging either to your finalizer method or to Dispose(bool disposing) method in order to catch situations when disposable object wasn't disposed correctly by calling Dispose() method. Most developers are looking for such information and you have a good place to inject it.

Answer 2

Something I've found useful in the past when implementing this pattern is to capture the stack trace during construction, and include this in the Debug.Assert message.

Knowing where the object was constructed can often be a help in tracking down the source of the issue, especially if the objects are created in one place and then need to be disposed of by some other object which takes over responsibility for their lifetime.

Answer 3

I'm in agreement with Brian, with the exception that I believe the Dispose(bool disposing) method should be abstract. My reasoning behind this, is that you must have a reason for inheriting from the DisposableObject type in the first place, in which case, having the method as virtual seems a little pointless, as it would be an optional implementation. If it's optional, why inherit from DisposableObject anyway?

Having it as virtual would be beneficial where you have some common code, but perhaps a better design would be:

protected virtual void Dispose(bool disposing)
{
  if (!Disposed)
  {
    DisposeInternal(disposing);
    Disposed = true;
  }
}

protected abstract void DisposeInternal(bool disposing);

That way you can assure that DisposeInternal is only ever called when you are actually disposing, and it doesn't need to set the Dispose flag, as this is handled by the outer call.

Answer 4

Personally I would have made Dispose(bool disposing) protected virtual rather than have 2 abstract methods ... for consistency with most unsealed disposable objects in the framework. Also, I recall FxCop was quite pedantic about the implementation of IDisposable.

If you still want to use use the two methods, I would suggest making at-least DisposeUnmanagedResources() virtual rather than abstract as a convenience to inheritors. I find it much more common for my disposable objects to dispose other objects rather than clean-up unmanaged resources of their own.

Answer 5

If this is restricted to wrapping unmanaged libraries, then it could be useful, but I wouldn't ever use it where there are no unmanged resources, it'll completely screw-up the garbage-collector- as one should not have a finalizer unless one absolutely needs it.

You must implement finalizers very carefully; it's a complex operation that can carry considerable performance overhead.

Answer 6

Adding a method for checking and throwing can clean up a lot of noise.

protected void VerifyDisposed()
{
    if (_disposed)
    {
        throw new ObjectDisposedException(GetType().FullName);
    }
}