0

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?
1
  • 2
    Did you ask about generics? Can't type parameters be used to avoid downcast?
    – Basilevs
    Oct 17, 2020 at 6:27

4 Answers 4

2
  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)

3

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.

1

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.

3
  • What do you mean by circumventing type safety, say with Foo? Yes, I can see you're not getting to restrict clients in the way envisioned/desired, but how does that break type safety? Doesn't Foo provide a Bar GetInnerObject? So, Foo is another wrapper for Bar inner objects, but what of it is a security breach circumventing the type system?
    – Erik Eidt
    Oct 17, 2020 at 10:34
  • I assume that the whole point of the whole "one interface per class" exercise is to guarantee that Foo.GetInnerObject always returns a Foo. So, I am using "type-safety" not in the sense of "the code allows you to circumvent the type system" but more like "the expressivity of the type system is insufficient for expressing the safety properties you want". Which is an issue that also exists in the original code, of course. So, it is technically safe with respect to the type in the code, but actually unsafe with respect to the "idealized type" in the programmer's head. I hope that makes sense. Oct 17, 2020 at 12:25
  • I replaced it with "desired safety properties" to avoid confusion. Oct 17, 2020 at 12:28
1

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.