Design dilemma: who should handle disposable parameter?

Question

If my class uses disposable resource in it's constructor (DbConnection if it matters) should I implement IDisposable in my class and dispose DbConnection object, or let user handle disposal of DbConnection?

Currently I implement IDisposable in my class, but now I see some possible negative effects: clutters class design, double disposal of DbConnection if used incorrectly. But there are also positive ones: simplier use is the major one (especially if you use multiple disposable parameters).

In the "wild" I see both approaches, so I can't decide..

Update: Thanks everyone for answers, which, in fact, showed that it's indeed not an simple choice sometimes. And it's hard to pick correct answer too. However I decided to stick to most simple one to follow in future. So final choice is: do not implement IDisposable.

Savannasavannah

If you do not implement IDisposable yourself, then the owner of the dbConnection that gets passed to you will have to know when you're done with it. Depending upon the particulars of your situation, that may be convenient or it may be awkward. I personally favor the approach of passing in a flag which says whether an object that's receiving an IDisposable should take ownership; coding is not difficult, and meaning is clear. Do you not like that approach? – Albertinealbertite

Answer 1

It should be disposed by whoever created it - the same scope as its creation. You create an object, you're responsible for disposing it. Simple as that.

Astonish

Answer 2

Using a disposable resource in a constructor is non-optimal and can lead to strangeness. You should instead be injecting a DbConnectionFactory into the constructor that is capable of creating connections on demand that you can use them and dispose them internally inside your methods.

If for some reason that pattern would be illogical for your scenario. The next best option would be to still move the usage of the disposable resource out of the constructor and have a method return a new type that is disposable and serves your purpose.

This would be similar to your service type class returning a data reader that needs disposed from a method. (Theoretically it could be a true data reader)

Digitalize

+1 for the factory, that's probably the cleanest and most unambiguous approach – Demarcation

Answer 3

I would either:

1. Not implement IDisposable; the calling code is responsible for both disposing of the passed-in object and ensuring it lives at least as long as the new object.

Or:

1. Implement IDisposable; the calling code relinquishes ownership of the passed-in object and should treat it as already disposed.

And clearly document which choice I made. Attempting to have the disposable object owned in multiple places is too likely to go wrong one day, IMHO.

Option 1 is for the scenario where you intend to pass the disposable object into several new objects, which will share access to it (a database connection sounds like it might be this). Option 2 is for when what you're doing is more like wrapping up the disposable object in another object with more (or more abstract) functionality, especially if you're returning it and so don't control its lifespan.

Prytaneum

Answer 4

There are some situations where an object will use a Disposable resource whose useful lifetime exceeds that of the new object that used it. There will be other situations where one will expect to hand off an object to an entity which knows nothing about any disposable items within it. Some types of object may find themselves used in both scenarios.

Consider a hypothetical IDisposable SoundPlayer type which will play an IDisposable SoundSource object and then dispose of itself when it's done. It's easy to imagine situations where one would want to be able to load a SoundSource once, play it multiple times, and then manually dispose of it afterward. It's also easy to imagine situations where one would want to load a SoundSource object and then "fire and forget" the player. The creator of the SoundSource can't Dispose it until the player's done, but will have no use for it afterward, so the most convenient course of action is for the SoundPlayer to dispose of it.

I would suggest that if both scenarios are at least plausible, you should either provide a factory method which transfers ownership of the IDisposable along with one that doesn't, or else you should have a factory method with a parameter that indicates whether ownership should be transferred. Note that passing IDisposables to constructors for classes that are supposed to consume and Dispose them is dangerous because Microsoft does not allow try/catch blocks around constructors invoked from field initializers and it's difficult to ensure things get disposed when a constructor throws. Having a factory method call the constructor makes error trapping much easier (though it's still a pain--especially in C#).

Edit--Addendum Another situation approach which is sometimes useful is to have an object either publish a Disposed event or accept a delegate in its constructor to be run when the passed-in object is no longer needed. I prefer the explicit delegate to the event, since using an event would imply that the event subscriber (the person creating the object that will temporarily hold the disposable resource) has an obligation to unsubscribe, which adds additional complication, especially if the object might be handed off to yet another object. Since by far the most common thing the caller will want to have done when the recipient of the object no longer needs it is for the object to simply be deleted, the simplest case is to simply have an option for the object recipient to handle disposal itself.

Albertinealbertite