Implement IDisposable correctly using object composition principle

Question

Update at the end

Is it possible to implement IDisposable pattern correctly while using object composition principle to promote code-reuse, reduce code duplication and hide verbose "official" implementation?

Rational

• Composition over inheritance is a good principle
• Implementing IDisposable correctly and thoroughly is very verbose and cumbersome

Proposal

Delegate the dispose logic to a dedicated class:

public class DisposeManager
{
    public Action Managed { get; set; }
    public Action Unmanaged { get; set; }

    protected virtual void Dispose(bool disposing)
    {
        // only dispose once
        if (disposed)
            return;

        if (disposing)
        {
            Managed?.Invoke();
        }

        Unmanaged?.Invoke();

        disposed = true; 
    }

    public void DisposeObject(object o)
    {
        Dispose(disposing: true);
        GC.SuppressFinalize(o);
    }

    public void FinalizeObject()
    {
        Dispose(disposing: false);
    }

    private bool disposed;
}

Implement IDisposable in user class in the following way:

public class DisposeUser : IDisposable
{
    public DisposeUser()
    {
        // using lambda
        disposeManager.Managed = () =>
        {
            // [...]
        };

        // or using member method
        disposeManager.Unmanaged = DisposeUnmanaged;
    }

    ~DisposeUser()
    {
        disposeManager.FinalizeObject();
    }

    public void Dispose()
    {
        disposeManager.DisposeObject(this);
    }

    private void DisposeUnmanaged()
    {
        // [...]
    }

    private readonly DisposeManager disposeManager = new DisposeManager();
}

Benefits

• much simpler to implement for user classes
• more explicit (managed, unmanaged)
• use composition
• remove the needs for multiple base classes all implementing the dispose pattern and creating code duplication

Questions

• Is it ever a good idea or more of a programmer fancy "improvement" idea ?
• I've made a decent number of research on the dispose pattern and implementation but never found someone suggesting such idea, any reason why?
• Any potential problems around hard refs, especially with Action capturing members, etc. that would prevent the actual user class to be collected correctly?
• Other thought?

Thanks!

Update

Drawbacks mention in answers

Mainly the drawbacks that were pointed in the answers are:

• probably overkill for most case because it's rare to have unmanaged resources to dispose
• doesn't cover IAsyncDisposable
• performance impact (delegates)
• Managed and Unmanaged action properties can re-set later
• Loose the well-known pattern knowledge for other developers
• Not much simpler/reducing code

New version

After using this base version in a project, I've made some improvements that may address some of the drawbacks mentioned. However this new version also accentuates some of them.

Changes

• expose list of actions to allow child class to easily add dispose logic
• add extensions methods to ease the add of disposable child(s)
• extension method to register to an event and unregister on dispose in one line

Those changes doesn't address much of the drawbacks mentioned, but I think it starts to bring enough benefits for the developer on a daily-basis usage to be worth it. Of course it's the case only if performance is not a concern and you need to dispose stuff in various places.

Answer 1

I think there is nothing wrong with your code, however I am unsure if it provides a significant benefit. What you are doing here is replacing one pattern with another, which is for sure simpler, but only marginally. So instead of one if and one SupressFinalize you need a FinalizeObject, DisposeObject a lambda. Not convinced this is a huge improvement, that might be the reason why you don't see it anywhere, although some kind of a dispose-helper is a frequent sight in many codebases. The problem with the new patterns is of course is that they are not known to people, so between a well-known pattern and a new pattern which is marginally better, I think the old pattern is preferable. Let's address your list of questions and benefits:

Benefits

• much simpler to implement for user classes

  As discussed, simpler, but not much simpler.

• more explicit (managed, unmanaged)

  True, however since there is no compile-time difference between the callbacks, my guess that after a while you will find that they are mixed up in the code base.

• use composition

  This is not a benefit. True, composition preferred to inheritance in most cases, but there is a reason for that, we should not just do composition for it's own sake.

• remove the needs for multiple base classes all implementing the dispose pattern and creating code duplication

  True, but then again, it has nearly as much code as before.

Questions

• Is it ever a good idea or more of a programmer fancy "improvement" idea ?

  Might be both. I don't see anything wring with using it, but forcing it on the codebase would be unwise.

• I've made a decent number of research on the dispose pattern and implementation but never found someone suggesting such idea, any reason why?

  Probably for the reasons above

• Any potential problems around hard refs, especially with Action capturing members, etc. that would prevent the actual user class to be collected correctly?

  I don't thinks so. There might be an issue with disposable ref-structs, but you would not want to use your code for them anyway. In some high-perf scenarios creating a bunch of new objects is a no-go, but it is rare.

Answer 2

Your code neither inherits nor composes "disposable-ability" with class DisposeManager. It is not being injected - the definition of composition. How many different "dispose" schemes does any class need anyway? Encapsulating DisposeManager implementation is not inherited, as a practical matter, without public methods. And you still must implement IDisposable. Inheriting means your object IS A "Disposable" type when an object should just BE "disposable", which is what an interface does. Besides subverting .NET GC design these notions conflate the complementary but distinct purposes of interface, composition, and inheritance.

Answer 3

I agree with vvotan's points that this utility could be useful, not much but still. However, I'd go further with the implementation. This means it should be separated into two disposers: Managed & Unmanaged that are both derived from the same base class.

This is how think it could be implemented:

public abstract class Disposer : IDisposable
{
    private bool disposed;

    private IDisposable Target { get; set;}
    
    private Action DisposeAction { get; set; }

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

    protected abstract void Dispose(bool disposing);    

    public void FinalizeObject() => Dispose(disposing: false);

    public class Managed : Disposer
    {
        public Managed(IDisposable target, Action disposeAction) => (Target, DisposeAction) = (target, disposeAction);

        protected override void Dispose(bool disposing)
        {
            if (disposed) return;
            if (disposing) DisposeAction?.Invoke();
            disposed = true;
        }
    }

    public class Unmanaged : Disposer
    {
        public Unmanaged(IDisposable target, Action disposeAction) => (Target, DisposeAction) = (target, disposeAction);

        protected override void Dispose(bool disposing)
        {
            if (disposed) return;
            DisposeAction?.Invoke();
            disposed = true;
        }
    }
}

• There is only one Action.
• There are two specialized disposers.
• Each disposer requires the target object (an IDisposable) and the Action to be specified via the constructor.

Not a huge difference but since we are talking about composition and separating concerns than it's a good thing to keep the discipline and write the code according to these principles.

---

For the potential user it's now even easier to use it because there is less room for mistakes:

public class DisposeUser : IDisposable
{
    public DisposeUser()
    {
        managedDisposer = new Disposer.Managed(this, () => 
        { 
            // [...]
        });             
    }

    ~DisposeUser()
    {
        managedDisposer.FinalizeObject();
    }

    public void Dispose()
    {
        managedDisposer.Dispose();
    }   

    private readonly Disposer.Managed managedDisposer;
}