IDisposable Part 2 – Subclass from an IDisposable class

For part 2 of my series on IDisposable, I’m going to focus on subclassing from our IDisposable resource.  We will build on our LicenseGenerator class from Part 1, extending it with some additional functionality.

When subclassing from an IDisposable resource, there are two possibilities for requirements in handling IDisposable correctly.  If we are not introducing any other unmanaged or managed resources that need to be disposed, we can handle this very simply.

We could just subclass from our IDisposable class, and make our new LicenseGenerator, but doing this without any extra thought would introduce a problem.  We would no longer correctly handle throwing an ObjectDisposedException if our new methods were used after the object was disposed.  In order to handle that correctly, when subclassing from an IDisposable resource, you must override the protected Dispose method with this syntax:

protected virtual void Dispose(bool disposing)

We do not need to do any disposal of resources (other than calling our base class dispose method), but we will need to track whether we’ve been disposed, so our other methods can throw the correct exception appropriately.

For example, say we want to subclass our LicenseGenerator to restrict it to just LicenseType.A licenses.  We also want to introduce a new method that needs to be checked for disposal.  In this case, we would write this class like so:

/// <summary>
/// A subclass of LicenseGenerator with none of its own managed resources
/// </summary>
class LicenseAGenerator : LicenseGenerator
{
    private bool disposed;

    // Just restrict this to LicenseType.LicenseA in the constructor
    public LicenseAGenerator()
        : base(LicenseType.LicenseA)
    {
    }

    /// <summary>
    /// Performs some additional work.
    /// </summary>
    public void PerformAdditionalWork()
    {
        // Every method should have this check, so we still need to handle this
        if(this.disposed)
        {
            throw new ObjectDisposedException("LicenseAGenerator");
        }
        // Do some work...
    }
    
    /// <summary>
    /// Releases unmanaged and - optionally - managed resources
    /// </summary>
    /// <param name="disposing"><c>true</c> to release both managed and unmanaged resources;
    /// <c>false</c> to release only unmanaged resources.</param>
    /// <remarks>
    /// If the main class was marked as sealed, we could just make this a private void Dispose(bool).
    /// Alternatively, we could (in this case) put
    /// all of our logic directly in Dispose().  I prefer sticking to the same pattern for sealed classes, however.
    /// </remarks>
    protected override void  Dispose(bool disposing)
    {
        // We DO still need to track whether we've been disposed correctly.
        this.disposed = true;
        // Call our base Dispose method
        base.Dispose(disposing);
    }
}

We can’t just ignore IDisposable, since we want to handle our exceptions correctly, but we do not need to do much in the way of checking or special handling with our override.

However, suppose we have a Logger class which is a disposable resource, and we want to create a subclass that uses our Logger to automatically log each and every request for a license.  Our “Logger” class has a “Log(string message)” method, as well as implementing IDisposable.

Adding Logger will cause us to now own a new managed resource, forcing us to change the way we handle IDisposable.  This new class will look something like:

/// <summary>
/// A subclass of LicenseGenerator which also contains its own managed resources
/// </summary>
class LoggingLicenseAGenerator : LicenseAGenerator
{
    /// <summary>
    /// Have we been disposed?
    /// </summary>
    private bool disposed;

    /// <summary>
    /// Our logger, which implements IDisposable and a method void Log(string)
    /// </summary>
    private Logger logger;

    /// <summary>
    /// Initializes a new instance of the <see cref="LoggingLicenseAGenerator"/> class.
    /// </summary>
    public LoggingLicenseAGenerator()
    {
        this.logger = new Logger();
    }

    /// <summary>
    /// Checks to see if a license is valid
    /// </summary>
    /// <param name="userName">Name of the user.</param>
    /// <returns>
    /// True if the license is valid, otherwise false.
    /// </returns>
    /// <remarks>This will also log our license requests</remarks>
    public override bool LicenseValid(string userName)
    {
        // Check to make sure we're disposed
        if (this.disposed)
        {
            throw new ObjectDisposedException("LoggingLicenseAGenerator");
        }

        // Use our base class to check the license, log, and return
        bool result = base.LicenseValid(userName);
        if (result)
        {
            this.logger.Log(string.Format("{0}: License granted to {1}", DateTime.Now, userName));
        }
        else
        {
            this.logger.Log(string.Format("{0}: License denied to {1}", DateTime.Now, userName));
        }

        return result;
    }

    /// <summary>
    /// Releases unmanaged and - optionally - managed resources
    /// </summary>
    /// <param name="disposing"><c>true</c> to release both managed and unmanaged resources;
    <c>false</c> to release only unmanaged resources.</param>
    /// <remarks>
    /// If the main class was marked as sealed, we could just make this a private void Dispose(bool).
    /// Alternatively, we could (in this case) put all of our logic directly in Dispose().
    /// I prefer sticking to the same pattern for sealed classes, however.
    /// </remarks>
    protected override void Dispose(bool disposing)
    {
        // Only do something if we're not already disposed
        if (!this.disposed)
        {
            // If disposing == true, we're being called from a call to base.Dispose().  In this case, we Dispose() our logger
            // If we're being called from a finalizer, our logger will get finalized as well, so no need to do anything.
            if (disposing)
            {
                this.logger.Dispose();
                this.logger = null;
            }
            // Flag us as disposed.  This allows us to handle multiple calls to Dispose() as well as ObjectDisposedException
            this.disposed = true;
        }
        // This should always be safe to call multiple times!  
        // We could include it in the check for this.disposed above, but I left it out to demonstrate that it's safe
        base.Dispose(disposing);
    }
}

Note that, now, we are reimplementing the entire IDisposable pattern in our Dispose(bool disposing) method.  We have a managed resource, so whenever we are “disposing”, we want to also call logger.Dispose().  If our finalizer (from the base class) runs, we can safely assume that Logger will get finalized as well, so we only dispose of it here.

About Reed
Reed Copsey, Jr. - http://www.reedcopsey.com - http://twitter.com/ReedCopsey

Comments

5 Responses to “IDisposable Part 2 – Subclass from an IDisposable class”
  1. Peter M. says:

    Interesting posts about IDisposable.
    I was wondering why you would create a new private bool disposed in your subclass. Why not make the member in the superclass protected? It would make your code a little smaller (which usually makes it more readable). Or are protected members considered a bad practice?

    • Reed says:

      Peter:

      It’s a matter of protection. The bool is a private implementation detail – technically, it’s only required if you have no way to tell via state that you’ve been disposed. That being said, by making it protected, a subclass could “undispose” you if it did something incorrectly. A protected property get method could be appropriate, as it prevents that case.

      However, adding a protected property getter is not part of the “official” pattern. I was trying to stick to the official pattern here, which doesn’t include that. In that case, using your own disposed is required. For example, the above pattern works if subclassing a framework IDisposable – in which case you can’t change the base class.

      -Reed

    • supercat says:

      The normal dispose pattern’s handling of the Disposed flag is a bit sloppy. It’s serviceable if the Disposed flag isn’t shared between derived classes, but would cause problems if the flag were exposed.

      The disposed flag would be better as an integer, which could then be updated and tested via Interlocked.Exchange in the public Dispose method; a Disposed property could be exposed publicly which would indicate whether the flag was set.

      One shouldn’t worry that a disposer which set the Disposed flag and threw an exception might prevent a finalizer from running, since classes which hold managed resources shouldn’t have finalizers in the first place.

      • Reed says:

        supercat:

        While I see your position on using an integer, I, personally, disagree with this idea. By exposing “Disposed” to any other classes, including subclasses, you open up the possibility of your code behaving inappropriately.

        Each class/subclass needs its own private tracking for safety purposes. This is the only way to guarantee proper tracking of disposal. I agree with this statement of yours: “would cause problems if the flag were exposed”. That is why I advocate that the disposed boolean should be private, not protected or public.

Trackbacks

Check out what others are saying about this post...
  1. [...] resources.  In this series, we’re going to build on our LicenseAGenerator class from Part 2, encapsulating inside of a class which will use it [...]



Speak Your Mind

Tell us what you're thinking...
and oh, if you want a pic to show with your comment, go get a gravatar!