3
\$\begingroup\$

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

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.

\$\endgroup\$
4
  • 2
    \$\begingroup\$ but never found someone suggesting such idea, any reason why? Disposing/Memory Management is a pretty sensitive to implementation topic. The solution for 99% cases looks like overkill (i have no unmanaged resources to dispose), for 0,99% looks like too generic because a very special implementation is needed. Also performance concern forces me to think about delegate allocations which can be simply avoided with implementing IDisposable natively. Finally, why there's nothing about IAsyncDisposable? \$\endgroup\$
    – aepot
    Jul 23 at 22:41
  • 1
    \$\begingroup\$ DisposeManager's Managed and Unmanaged properties feel so wrong. Allowing consumers to define their own method without any constraint can be really dangerous. For example they can call ReRegisterForFinalize and with that they can resurrect the object. \$\endgroup\$ Jul 28 at 7:29
  • \$\begingroup\$ @aepot I see your point about "too generic" and I also asked this question myself. Therefore I was thinking to simply implement IDisposable directly were needed and ignore unmanaged. However, what if a derived class needs to dispose unmanaged resources but the base class already implement IDisposable with a simple public virtual void Dispose() that doesn't deal with unmanaged? \$\endgroup\$
    – cid
    Aug 16 at 9:17
  • \$\begingroup\$ @PeterCsala Agree, would be better to give them a ctor time. Is it what you suggest? \$\endgroup\$
    – cid
    Aug 16 at 9:18
5
+25
\$\begingroup\$

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.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for the detailed answer. I agree about the well-known pattern and also the not much simpler. I've tried using this in a project and added some functionalities that make this more useful and provide more benefits I think. I will edit by question to share it. \$\endgroup\$
    – cid
    Aug 16 at 9:56
1
\$\begingroup\$

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.

\$\endgroup\$
1
  • \$\begingroup\$ From my understanding Composition over inheritance doesn't necessarily implies ctor injection. Taken from the C++ example they do for example Player() : Object(new Visible(), new Movable(), new Solid()). Besides this article, I think it's often a good idea to group a set of related behaviors in a sep. class to promote easier code reuse elsewhere. Even if it's not up to the point where it is injected. For the rest I can see your point about the possible confusion of this solution. \$\endgroup\$
    – cid
    Aug 16 at 9:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.