Question

I have the following code example:

public interface IRepository {
   // Whatever
}

public class SampleRepository : IRepository {
   // Implements 'Whatever'
}

public class NHibernateRepository : IRepository, IDisposable {

   // ...

   public void Dispose() { ... }
}

Now - is that really bad? I'm not sure, but this seems to be pretty the same as not marking the destructor of the base class virtual in C++.

I don't want to make the IRepository interface implement IDisposable, because that would bring unwanted complexity and bunch of classes which would also have to implement IDisposable.


How should this case be handled?

I'm sure this can happen to any type hierarchy - when one of the derived type has to manage disposable resources.

So what should I do - pull the IDisposable up to the very first interface or leave it as it and hope user would distinguish disposable and non-disposable repositories?

Thank you.

Was it helpful?

Solution

What you've described is okay with one important proviso: an object of a type which has added 'IDisposable' to its base must never be passed to a consumer which may end up holding or persisting the object for an unknown duration.

Basically, the owner of an IDisposable object must(*) either take care of disposing the object itself, or hand the object off to a new owner which can accept and honor the responsibility. Initially, IDisposable objects are generally "owned" by their creator, but hand-offs are common. A reference to an IDispoable object can be given to another object without transferring ownership; in that scenario, the owner is still responsible for disposing the object when it is no longer needed. For that to happen, the owner has to know the object is no longer needed. The most common patterns are:

  1. The object is passed as a parameter to a method; the object hosting the method will not use the object after the method returns.
  2. The object is passed as a parameter to a method which will store it in some object's field, but that object can later somehow be requested to destroy the reference.
  3. The disposable is passed as a parameter to a method which is hosted by an object to which the owner of the disposable object holds all references; the hosting object won't use the disposable object unless requested to, and the owner of the disposable object will know if it'll never issue any more such requests.

If one of those patterns applies, you may be in good shape. If not you may be in trouble.

OTHER TIPS

Yes, I'd say this is bad - because you can't use all repositories in the same way.

I would personally make the repository interface extend IDisposable - it's easy enough to implement it with a no-op, after all.

This is exactly the same choice as is made by Stream, TextReader and TextWriter in the main framework. StringWriter (for example) doesn't need to dispose of anything... but TextWriter is still disposable.

This all comes about because IDisposable is in some ways an odd interface: it doesn't convey something that is offered to callers... it conveys something which is required of callers (or at least, strongly encouraged with possible issues if you don't go along with it).

The only issue you might have is if you are using some type of factory, Inversion of Control, or Dependency Injection pattern / framework. If you ever want to use the object as an interface, you will never be able to do this:

IRepository repo = Factory.GetRepository();
repo.Dispose();

You may want to introduce a INHibernateRepository that implements IDisposable. Because IDisposable is usually such a low level operation, there's not a huge issue with making your interfaces implement this interface.

No, do not "pull the IDisposable up". Keep interfaces atomic, simple and independent.

Unless you could argue that being IDisposable is a fundamental trait of IRepository (and SampleRepository shows it isn't) it there should be no derivation between the two.

That seems perfectly fine to me. What's the problem?

First, to answer your question. The dispose pattern is different from a C++ destructor. A Dispose method is intended to dispose of resources contained by the class, instead of disposing of the class itself.

The reason for marking C++ destructors as virtual does not exist in .NET because every instance of a reference type has a sync block which contains run time type information. Using this, the garbage collector can appropriately reclaim the correct amount of memory.

As for extending IRepository with IDisposable, that would be a quick fix which would be acceptable in the vast majority of cases. The only objection that I can see is that extending the interface will require all derived classes to implement the interface. On the face, it may seem easy to implement an interface with NOPs (multiple times perhaps), but you shouldn't have to. But, I can offer an alternative.

Instead, consider using an abstract base class which implements the Dispose Pattern. This would follow the structure of the "standard" implementation of the dispose pattern.

public abstract class Repository : IDisposable  
{ 
    public void Dispose() { Dispose(true); }
    protected virtual Dispose(bool disposing) {  } 
}

public class NHibernateRepository : Repository { /* Impl here, add disposal. */ }

public class TestRepository : Repository { /* Impl with no resources */ }

Take a look at the Dispose Pattern Guidelines written by Microsoft to see a more detailed example.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top