tne
tne

Reputation: 7261

Conservative IDisposable marking on interfaces

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

Answers (4)

supercat
supercat

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

tne
tne

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

Jodrell
Jodrell

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

Marc Gravell
Marc Gravell

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

Related Questions