Friday, July 22, 2016

Unknown disposables: Disposing of non-disposable interface instances

In coding, it's a good practice to use factory methods (or some other creational design pattern) to abstract away creation of class instances. In most of the cases, the "creator" would return not the actual type being created, but the abstraction - the interface.
To emphasize the problem, let's consider the below example:

For simplicity, let's assume that we're dealing with a factory method, and the interface the factory method returns is called "IContract". Note, that IContract does not implement IDisposable. There are two different implementations of IContract - Impl and DisposableImpl. So here comes the interesting part, the DisposableImpl implementation does implement IDisposable.

So now, if one uses the factory method "Create()" to instantiate IContract, it may get a disposable instance back, and won't ever even know about it:

public class ContractFactory
{
    public IContract Create()
    {
        return new DisposableImpl();
    }
}

Obviously, the responsibility of disposing of the disposable instance is on the caller. A lot of people won't even notice the problem and just use the IContract and never worry about freeing up resources as early as possible. Yes, GC will eventually take care of the unnecessary resources, but that'll happen later than the actual variable becomes unnecessary - during finalization.

So one pattern a client can use is the following:

{
    IContract contract = null;
    try {
        contract = new ContractFactory().Create();
    // all the usage of the contract instance happens here
    }
    finally
    {
        IDisposable disposableContract = contract as IDisposable;
        if (disposableContract != null)
        {
            disposableContract.Dispose();
        }
    }
}


This will ensure safety in case of both disposable and non-disposable implementations.
Another way to go, would be to have a wrapper class, encapsulating the disposal logic and being a disposable itself, as follows:

public sealed class DisposableInterface<T> : IDisposable where T : class
{
    private readonly T interfaceInstance;

    public T Instance
    {
        get { return this.interfaceInstance; }
    }

    public DisposableInterface(T instance)
    {
        if (instance == null)
        {
            throw new ArgumentNullException("instance");
        }
       
        this.interfaceInstance = instance;
    }

    // A better implementation would be the usage of the Dispose pattern here.
    // Not doing that here to keep the sample simple.
    public void Dispose()
    {
        IDisposable disposableInstance = this.Instance as IDisposable;
        if (disposableInstance != null)
        {
            disposableInstance.Dispose();
        }
    }
}

Now, the caller could use the IContract as follows:
{
    using (DisposableInterface<IContract> contract = new DisposableInterface<IContract>(new ContractFactory().Create()))
    {
        IContract instance = contract.Instance;
        // all the interaction now happens with the "instance" reference
    }
}

As you can see, the second example is cleaner and nicer.
Now, as you're familiar with the problem, doesn't it raise a natural question: "should this become a pattern and be used all over the place with factory methods, as one can't know whether the actual implementation the factory methods returns will be disposable or not?"....
Think about this and leave any comments you may have.