Search code examples
c#.netapi-designidisposable

Implementing IDisposable in an API


I'm currently writing a fairly large API and am confused as to how I should be implementing IDisposable. In a simplified scenario:

I have a Socket in one of my classes (call it A), which will obviously need disposing of, however that class only has internal constructors and hence my user would only be able to create an instance of it by creating an instance of a higher level object (call it B) which would in turn instantiate an A and thus take ownership of the new object A.

My intuition suggests to make both objects A and B IDisposable so that when the user disposes of our owning object B in their application, A is disposed of and the socket gets correctly disposed of as well. However, since this is an API, it would allow my user to dispose object A and the socket without disposing of the owning object B and hence could cause themselves some major issues.

So how should I proceed? As far as I can see my only options are bad options:

  1. Implement IDisposable as intuition would have it and tell my user not to be stupid in the documentation. (Very bad)
  2. Implement IDisposable as intuition would have it but add some hideous disposal checking/exceptions throughout the application just in case (although in the case of my application disposing of whole subsections would be fatal to the application anyway).
  3. Only implement IDisposable in object B and instead add an internal Dispose method to safely dispose of A and the socket. (seemingly the best idea)

I'm not really sure if I have any other options so I really hope someone can advise me.


Solution

  • My intuition suggests to make both objects A and B IDisposable so that when the user disposes of our owning object B in their application, A is disposed of and the socket gets correctly disposed of as well. However, since this is an API, it would allow my user to dispose object A and the socket without disposing of the owning object B and hence could cause themselves some major issues.

    Make both implement IDisposable is probably the best idea and clearly illustrates the fact that the code instances need to be cleaned up. Instruct the consumer of the API to call Dispose on the root/owning object (B). To get around the issue of the consumer of the API accidentally also calling Dispose on the owned (A) instance you could do the following.

    Hide it

    Do not expose the owned reference A from root/owning object B or provide a limited number of pass through methods that call into the root and pass through to the owned instance and then back again. This is fine if you are talking about real limited number of methods/properties that will not change.

    Interfaces

    Wrap the socket access using a wrapper and interface (I am assuming here that the socket is a .net socket class but if its your own object implementation around the socket class then your class is the wrapper already and no need to create another wrapper around that). The wrapper should implement IDisposable and interface should only expose what you want the client to have access to. I am not sure where you create instances of the socket but they should be created by either the owned object OR using a factory pattern so that the owner relationship is established asap after creation. You can keep the class definition internal and only expose an interface to the API.

    In my opinion this is the best way to do it and allows for flexible changes in the future if you need to extend your API.

    public class Owner : IDisposable
    {
        private SocketWrapper _wrapper;
        public ISocketWrapper SocketAccess { get { return _wrapper; } }
        public void Dispose()
        {
            if (_wrapper != null)
                _wrapper.Dispose();
        }
    }
    
    public interface ISocketWrapper
    {
        // exposed properties/methods
    }
    
    
    internal class SocketWrapper : ISocketWrapper, IDisposable
    {
        public void Dispose()
        {
            // dispose socket
        }
    }