CODEWITHSUNDEEP
c#.netidisposable
mdn
mdn

Reputation: 139

Implement the inhertited IDisposable pattern with no CA warnings

I am dealing with following case. I have a base class that have the IDisposable pattern, it means that it have the public virtual Dispose() method and the protected virtual Dispose(bool) method. But I am not able to implement this patter in the derived class in such way that I will not receive any CA warnings. Please consider following example:

public class UtilizeClass : IDisposable
{
    private MyData data;

    public UtilizeClass()
    {
        data = new MyData();
    }

    public void Dispose()
    {
        data.Dispose(); // Cannot use Dispose(bool) because it is protected.
    }
}

public class MyData : Base, IDisposable
{
    // Here we have some managed resources that must be disposed.

    // How to implement the pattern?
}

public class Base : IDisposable
{
    public virtual void Dispose() { }
    protected virtual void Dispose(bool disposing) { }
}

All the time I receive contradictory CA warnings in the MyData class. For example: delete the Dispose() and move it logics to Dispose(bool).

Thank you very much for answer and help.

Upvotes: 1

Views: 156

Answers (3)

Jon Hanna
Jon Hanna

Reputation: 113322

If Base is your own class, then don't implement this anti-pattern.

The two-fold dispose (one if disposing is true, one if its false) is used when a class contains both managed resources that must be disposed (e.g. a Stream object that should have it's own Dispose called) and unmanaged resources that must be cleaned-up.

This is a bad idea. Instead have all your classes fit into one or two categories:

A. Classes with only unmanaged resources. Ideally only one per class:

public sealed class HandlesUnmanaged : IDisposable
{
  private IntPtr _someUnmanagedHandleOfSomeKind;
  public string DoSomething(string someParam)
  {
    // your useful code goes here;
    // make it thin, non-virtual and likely to be inlined
    // if you need to extend functionality, but it in a
    // containing Disposable class, not a derived class.
  }
  private void CleanUp()
  {
    //your code that cleans-up someUnmanagedHandleOfSomeKind goes here
  }
  public void Dispose()
  {
    CleanUp();
    GC.SuppressFinalize(this);//finaliser not needed now.
  }
  ~HandlesUnmanaged()//not called if already disposed
  {
    CleanUp();
  }
}

Ideally you won't even need any classes like this, but use SafeHandle which does that for you.

B. Classes with one or more managed resources that need to be disposed:

public class NoUnmanaged : IDisposable
{
  private HandlesUnmanaged _likeAbove;
  private Stream _anExampleDisposableClass;
  public virtual void Dispose()
  {
    _likeAbove.Dispose();
    _anExampleDisposableClass.Dispose();
  } 
  /* Note no finaliser, if Dispose isn't called, then _likeAbove's
  finaliser will be called anyway. All a finaliser here would do is
  slow things up and possibly introduce bugs.
  */
}
public class DerivedNoUnManaged : NoUnmanaged
{
  Stream _weMayOrMayNotHaveAnotherDisposableMember;
  public override void Dispose()
  {
    //note we only need this because we have
    //another disposable member. If not, we'd just inherit
    //all we need.
    base.Dispose();
    weMayOrMayNotHaveAnotherDisposableMember.Dispose();
  }
}

In all, we've either got simple unmanaged-owning classes that do the same thing in their Dispose() and their finaliser, except the former calls GC.SuppressFinalize, or we've got simple non-unmanaged-owning classes that just Dispose() everything they need to dispose, including a call to base.Dispose() if necessary, and don't have finalisers. No needs to split off the logic into two types within the same class. No risk of finalisers calling something that's been finalised, or forcing more than necessary into the finalisation queue.

And ideally, you never even do the first type at all. Just the second type.

If you're forced into it by inheriting from another party's class, then just do:

public MyClass : Base
{
  Stream _forExample;
  public override void Dispose(bool disposing)
  {
    if(disposing)
    {
      _forExample.Dispose();
    }
    base.Dispose(disposing);
  }
}

Don't handle the disposing == false case yourself, because don't have unmanaged resources mixed in there.

Upvotes: 2

SLaks
SLaks

Reputation: 887767

The base class should have a public void Dispose() (non-virtual) that calls Dispose(true).

The derived class should simply override protected virtual void Dispose(bool disposing).

Upvotes: 0

Pete
Pete

Reputation: 11495

Your base class shouldn't have void Dispose() be virtual, it should be implemented and invoke the virtual void Dispose(bool disposing) as part of its implementation.

For more detail, and an alternate API that's slightly clearer, check out:

http://haacked.com/archive/2005/11/18/acloserlookatdisposepattern.aspx

Upvotes: 2

Related Questions