Is Interface inheritance a bad practice?

Question

I was wondering if the following code shows a bad practice (about interface inheritance):

public interface IFoo : IDisposable
{
    void Test();
}

public class TestImpl : IFoo
{
    public void Test()
    {
        // do something
    }

    public void Dispose()
    {
        // disposing my dependencies
    }
}

public class FooFactory
{
    public IFoo CreateFoo()
    {
        return new TestImpl();
    }
}

public class Client
{
    public void Main()
    {
        FooFactory factory = new FooFactory();
        using(IFoo foo = factory.CreateFoo())
        {
            // do stuff then auto-dispose
            foo.Test();
        }
    }
}

Here, I wanted two things: 1. Use the C#'s using statement so I can dispose my concrete object's dependencies elegantly (no need to cast anything to IDisposable). 2. Use a factory method to create a concrete object, so I can work only with the interfaces.

Separated, both are known good practices, but together? I didn't find anything saying it's wrong here http://msdn.microsoft.com/en-us/library/ms229022.aspx (MSDN - Interface Design), but I think it doesn't sound good...

I would appreciate any comment about this. If it's a bad practice, what kind of issues I would run into using this approach?

Thanks!

Answer 1

Technically everything is fine with this solution.

From a semantic point of view you could argue that the need of IDisposable is an implementation detail and has nothing to do with the actual contract of your class.

What you can do is cast the object to IDisposable and call Dispose() only, if it implements IDisposable:

var disposable = foo as IDisposable;

if (disposable != null)
    disposable.Dispose();

But in this case I would go with interface inheritance as you suggested it. It isn't the cleanest aproach, but does the job for you perfectly and needs less code.

Answer 2

In the wild, there is no such thing as a definitely good or bad practice, each has its upsides and downsides.
A practice becomes good or bad only when it is applied to a concrete problem (read: not IFoo).

public bool IsGood<TContext>(IPractice practice)
    where TContext : RealWorldApplication, new();

So, instead of trying to find guidance on MSDN, ask yourself the following question:

Does this solve the problem and make the code simpler to read?

If the answer is yes, go with it.

Personally, I like the approach. If the contract for IFoo requires proper disposal (think: IGraphicalObject, IResource, IConnection), it's a very good design decision to make this explicit.

Don't be trapped by dogma: programming with interfaces makes you less dependent on concrete implementations, but programming only with interfaces can be a nightmare. Be reasonable, that's it.

Update

You can use Google to find all interfaces that inherit from IDisposable in .NET Framework itself:

intitle:interface "inherits idisposable" site:msdn.microsoft.com

As I said above, this is normally done for entities that are known to consume resources, such as database connections, unmanaged object wrappers, graphical objects, et cetera.

Imposing implementing Dispose() on a class that has nothing to dispose of would be a bad idea.
In this case it's better to leave it optional and check for IDisposable support, as suggested by Peter.

Answer 3

I think it is correct. Your factory hides what class is actually instantiated so IFoo have to inherit IDisposable for the client to be able to call Dispose(). With this class the client can - and should - be totally unaware of the actual class instantiated. All relevant information should be carried by the interface - including the need for disposal.

Answer 4

This is perfectly valid. That's exactly what interfaces are for. They define contracts. A contract can be built using several sub contracts.