IDisposable implementation in C# - is optional disposal of an injected IDisposable OK?

StackOverflow https://stackoverflow.com/questions/19486168

  •  01-07-2022
  •  | 
  •  

Pergunta

if I have fairly a standard abstract class which allows injection of an IDisposable instance. However, some classes inheriting from this class should NOT dispose the injected repository, where as others should. The obvious solution is to have a constructor:

public abstract class WorkspaceViewModel : IDisposable
{
    readonly bool _cascadeDisposeRepository;
    protected WorkspaceViewModel(IRepository repository, bool cascadeDisposeRepository=true)
    {
        _repository = repository;
        _cascadeDisposeRepository = cascadeDisposeRepository;
    }

Edit:

I also had a constructor with

    protected WorkspaceViewModel()
        :this(new RepositoryA(), true){} 

End Edit

and then implement the Dispose method in the recomended microsoft manner

protected virtual void Dispose(bool disposing)
{
    if (!_disposed)
    {
        if (disposing)
        {
            if (_cascadeDisposeRepository) { _repository.Dispose(); }
            .....

However, I have never seen IDisposable implemented in this way, and was wondering if it is bad practice (and if so, why, and what other solutions are preferable).

Thank you for your thoughts.

edit: Mark's comment made me realise the preferable implementation may be without the parameter-less constructor, forcing any classes inheriting from WorkspaceViewModel to create and dispose their own instances (and choose to implement IDisposable), while removing IDisposable from the implemented interfaces for WorkspaceViewModel.

Foi útil?

Solução

Since you have no handles on unmanaged resources I would suggest removing IDisposable altogether.

Since your class is a repository I suspect it uses a database connection which probably indirectly has a handle on an unmanaged resource - so just remember to wrap it in a using:

using(var myConn = new Connection(connectionString))
{
}

Then you can let the beauty of managed code worry about what to destroy and keep-alive - you don't even need to think about it.

Outras dicas

The last entity to "use" an object which implements IDisposable, either by accessing it directly or by passing it to some other entity for known-duration temporary use, should call Dispose on it. If Moe is passed an IDisposable from some other entity which will inherently know when Moe is done with it, then Moe can generally expect that other entity to take care of it and need not call Dispose itself. Indeed, it the other entity might have use for the IDisposable after Moe is done with it, then Moe must not call Dispose itself. If Moe is passed a reference of either interface or base-class type, there's no need for the interface or base class to implement IDisposable; even though a passed-in instance might be of a derived type which implements IDisposable, that's not Moe's concern. The entity which created that instance should know that it's a type which implements IDisposable and deal with it.

The place things get tricky is with factory methods. If there's any realistic possibility that a factory method might return an object which implements IDisposable, and that the caller of that method would be the only thing that would know when the object is no longer needed, then the return type of that method should itself implement IDisposable. Because the non-generic IEnumerator interface did not follow that pattern, code which uses the non-generic IEnumerable interface is required to check whether each object returned by GetEnumerator() implenments IDisposable and call Dispose on it if so; this is less convenient and slower than would be calling IDisposable unconditionally had it been inherited by IEnumerator [even if only 99.9% of IEnumerator implementations would have a do-nothing Dispose method, calling a do-nothing method which an interface is known to support is faster than checking whether an interface supports a method]. Note that having the factory method's return type implement or inherit IDisposable wouldn't add any responsiblity to the caller--it would merely make it easier for callers to carry out the responsibility they will have regardless.

Licenciado em: CC-BY-SA com atribuição
Não afiliado a StackOverflow
scroll top