Is it an issue to have multiple almost identical interfaces?

Question

Is it an issue to have multiple almost identical interfaces?

In our company we have 9 identical interfaces for 9 document types, like: Book, Contract, Person, etc.

interface IBookInner    { Book     GetInnerObject();}
interface IContractInner{ Contract GetInnerObject();}
interface IPersonInner  { Person   GetInnerObject();}

class Book    :IBookInner{...}
class Contract:IContractInner{...}
class Person  :IPersonInner {...}

They all have same method but returing type differs. And each document type realizes its own interface (soon I will add one more).

I've suggested to replace all of them with one like:

interface IInner {Object GetInnerObject();}

But colleagues answered, that it's bad idea, because with one interface we need to use dynamic cast (which can cause exceptions), while with separated interfaces we have more stable code.

Is it an issue to have an unique interface for every document type?

I see these pluses of having one interface instead many (almost identical)

1. Less code (not much less, but still)
2. If we want to change behavior of our documents, we need to fix 1 interface + 9 realizations instead of 9 interfaces + 9 realizations.
3. One interface is more abstract and less tied to business logic (compared to those interfaces for every document type)

The minuses are:

1. With one interface we need to use dynamic cast which can potentially cause runtime exceptions

(Also I should say, that in our company we are trying to use SOLID in our code. I'm not very experienced with SOLID, but I have a feeling that having such interfaces for every type is against dependency inversion principle.)

So my questions are:

1. Is it an issue to have multiple almost identical interfaces?
2. Which is better to have one unified interface or interfaces for every document type?
3. If we look from SOLID perspective, is it good or bad choice to have multiple almost identical interfaces (to avoid dynamic cast) instead of one?

Answer 1

1. Yes, having a bunch of interfaces that only vary by return type is gross. Particularly anything that returns “Inner” is a huge code smell, as it indicates some sort of encapsulation leakage.
2. One for each type is marginally better. Having an interface that just returns object is very fragile and will increase implicit coupling wherever it is used.
3. Arguably, the Interface Segregation Principle favors more, specific interfaces. But I expect in practice you’re comparing a douche and a poop sandwich.

A better compromise would be to have one generic interface that you can then parameterize based on return type. No casting, no boilerplate templating.

(But seriously, consider a design where the entire pattern isn’t necessary in the first place)

Answer 2

It is hard to tell but it looks like a Pavlov reaction to slap an interface on every new class in a systematic manner. Interfaces are supposed to isolate certain atomic behavior and you would expect this to be different for every new class. "Inner" does not represent a behavior.

So first I would want to know what is the point of having these inner interfaces. I suspect they were introduced for the wrong reason and are now just maintained because this is how you do things.

Answer 3

Since your 9 interfaces do not need to know anything about the concrete type, we can make them parametrically polymorphic in the type:

interface IInner<out T> { T GetInnerObject(); }

class Book     : IInner<Book>     { /* … */ }
class Contract : IInner<Contract> { /* … */ }
class Person   : IInner<Person>   { /* … */ }

This is not as safe as the separate interfaces because a malicious client could do something like this:

class Foo : IInner<TotallyUnrelatedType> { /* … */ }

We can use F-bounded quantification to further restrict the type:

interface IInner<out T> where T : IInner<T>
{
    T GetInnerObject();
}

However, even with this, it is still possible to circumvent, but requires more work. We can no longer use a totally unrelated type, we must use a type that also implements IInner:

class Bar : IInner<Bar> { /* … */ }
class Foo : IInner<Bar> { /* … */ }

So, this allows a malicious client to still circumvent our desired safety properties, but it is highly unlikely that code like this is written by accident. So, if you are not worried about someone actively attacking your code base, but about honest programming mistakes, this is good enough™️.

And by the way, the same flaw exists in the original solution, there is nothing stopping me from writing

class Book : IContractInner { /* … */ }

In programming language type theory, there is a concept that would help us here, called a MyType. In the context of C#, you can roughly think of it as typeof(this). If C# had a MyType feature, we could make this type-safe like this:

interface IInner { typeof(this) GetInnerObject(); }

class Book     : IInner { /* … */ }
class Contract : IInner { /* … */ }
class Person   : IInner { /* … */ }

But, this feature doesn't exist in C#, so we have to make do with the F-bounded quantified parametrically polymorphic interface above.

However, as mentioned in Telastyn's answer, all the ways in which we can make this better are moot, because it looks like there is a deeper design problem which, when solved, make those interfaces unnecessary.

Answer 4

Just because a method has the same name doesn't mean it should be in the same interface. You didn't complain about needing to cast to one of the interfaces, so I get the feeling you could rename the methods to getBook, getContract, and getPerson without affecting much.