Reputation: 7261
In a situation where implementations of an interface may or may not need to be disposed of, is it a good idea to require that all implementations must be disposed of by marking the interface itself as IDisposable
?
This forces users of this interface to always dispose of the objects even though it might be unnecessary for certain implementations. The goal is obviously to ensure that implementations are always seamlessly swappable. The alternative is to leave it to the user to decide, which might seem more correct, but more difficult to communicate. On the other hand, considering a user that only has a symbol declared with the type of the interface, it might seem more correct to require that it be disposed of unconditionally. Is there a trade-off or a fallacy in this reasoning?
If the question is too dependent on the situation, I'd like to frame it in the context of textbook repositories (of the repository design pattern), where most known implementations will need to be disposed of.
Upvotes: 4
Views: 173
Reputation: 81159
In deciding whether IFoo
should implement IDisposable
, the critical question shouldn't be whether a substantial fraction of types implementing IFoo
will need disposal, or even whether a substantial fraction of the object instances implementing IFoo
will need disposal, but rather whether it's likely that any non-trivial fraction of the instances which are returned by a factory method whose return type is IFoo
will need to be disposed by the caller of that factory method.
Consider IEnumerator
. Only a tiny fraction of the classes which implement it require disposal (the fraction probably increased significantly with the introduction of iterators), but the vast majority of instances will be obtained by a call to factory method IEnumerable.GetEnumerator()
, whose return type is IEnumerator
. Code which calls IEnumerable.GetEnumerator()
will often have no knowledge of whether the returned object will require disposal or not; for semantic correctness, unless code knows that the types of IEnumerable
it's using won't return instances that require disposal, it must for correctness try-cast the returned implements to IDisposable
and, if the result is non-null, dispose them.
When Microsoft implemented IEnumerator<T>
, they realized that the burden of consumers having to try-cast enumerators to IDisposable
was far greater than would be the burden of implementers having to implement a do-nothing Dispose
method. Semantic correctness requires that consumers call Dispose
on any enumerator which implements IDisposable
, and unconditionally calling a do-nothing Dispose
method on a type which is known to implement it is easier and faster than testing whether a type which might implement IDisposable
, does so.
Even if only 1% of IEnumerator<T>
implementations implement IDisposable
, the vast majority of consumers must, for semantic correctness, assume that any of them might. Having IEnumerable<T>
inherit IDisposable
does not impart any obligation on consumers that they wouldn't otherwise have--it just makes it easier for them to carry out obligations that they would have in any case.
Upvotes: 2
Reputation: 7261
Having recently pondered on the issue again, I would like to share another guideline / rule of thumb that, if applicable, may make the decision easier.
What nobody considered yet is the ratio of users / implementations:
If there are more users than implementations, and especially if the implementations are all under your control, then consider marking the interface IDisposable
and force the users to always dispose even if it's not strictly necessary. As long as this feels appropriate for the use-case at hand, it will most likely make the life of the users simpler: they won't need to check if they need to dispose or not, they'll do it systematically. The drawback is that you will have to implement dummy Dispose
methods for those implementations that don't require it. Unless you have peculiar performance requirements, this call is perfectly fine (I would imagine the only overhead is the virtual table lookup, the method itself is likely optimized-out).
If there are more implementations than users, for example in the case of a plugin system, then consider verifying if the objects implement IDisposable
dynamically in your core, so that implementations don't ever have to implement dummy IDisposable
methods. The drawback is that you require additional logic in your core. Overhead is that of a cast & null-check.
tl;dr: We often assume there will be more users than implementations. This is not always the case.
Of course, this is just one dimension of the problem that you can use to decide; the other dimension is that of the ratio between implementations that need to be disposed of and implementations that don't. See the other answers for a discussion on this aspect.
Once you've evaluated each, take all considerations at once, salt with your opinion, and make your decision.
Upvotes: 0
Reputation: 35716
So, you are exposing an interface
ISomeInterface
If you intend to use this interface to trigger the disposal, i.e. you want to call Dispose
, then, it should implement IDisposable
.
ISomeInterface : IDisposable
However, consider that the composition of the ubiquitous IDisposable
will certainly cause 3rd-party users of ISomeInterface
implementations to call Dispose
themsevles, either explicitly or in using
blocks.
If you are not intending to call Dispose
yourself, leave the choice to the concrete class implementer. They alone can decide if disposal will be necessary in a given situation.
Now I can quote an authoritative source,
not to demand it unless it is overwhelmingly likely that most implementations will require cleanup
I would suggest that in the example of a Stream
it is "overwhelmingly likely."
Upvotes: 1
Reputation: 1062855
If the question is too dependent on the situation, I'd like to frame it in the context of textbook repositories (of the repository design pattern), where most known implementations will need to be disposed of.
If you are writing library code that instantiates objects and is responsible for their lifetime, then the ideal scenario would be to handle IDisposable
, but not to demand it unless it is overwhelmingly likely that most implementations will require cleanup; for example by using is
or as
at the point when your code is done with the unknown object. It is rarely as simple as this but for example:
IFoo obj = (IFoo)Activator.CreateInstance(unknownType);
using(obj as IDisposable)
{
// TODO: code
}
Upvotes: 2