29

I''m working on a C# library where the API provides several public interfaces and a single concrete factory class (itself an interface implementation). This factory provides implementations of the various interfaces. Other than the factory, none of the actual implementations are available to the user.

As is the nature of using interfaces, this decouples my users from being concerned with how I variously decide to refactor the implementations. I know that's good practice in itself, but I do feel I'm maybe being a little restrictive not making the implementations public.

Is this common/accepted practice? What are the pros and cons of this approach?

12
  • 1
    One thing to note: If your parameter types are also interfaces, it allows your consumers to implement them themselves. Which might be good or bad. Commented Sep 19, 2022 at 23:50
  • Tangentially related, I just saw this video a couple days ago that gives a performance-based reason for closed/sealed implementations.
    – Daevin
    Commented Sep 20, 2022 at 18:12
  • 9
    I see a close vote on this question, but definitely disagree. This is the exactly type of question which SE.SE is built upon and intended for. Commented Sep 20, 2022 at 18:48
  • 1
    Comment, as it is totally unrelated to accepted answer (and hence can't be an answer to the question): exposing interfaces let users of the library to test their own code with unit tests, exposing non-creatable classes requires users of the library to wrap/proxy calls with intermediate interface to be able to test their own code that uses the library... Commented Sep 20, 2022 at 22:06
  • 1
    My rule of thumb is an interface is public but an implementation is internal. Then you provide a mechanism to register the implementation with DI (services.AddMyThing();). The slight hiccup is that when unit testing, you have to use [InternalsVisibleTo] (or the csproj equivilent), to allow the unit test project to see the implementation class.
    – Neil
    Commented Sep 21, 2022 at 14:08

3 Answers 3

23

While this may be an opinion-based question, and the real answer is the same as with many endeavors in software design and development ("it depends"), I'm going to say yes, do not expose these implementation details by adhering to the Principle of Least Privilege. If and only if you find your design has good reason to expose the implementation for extension -- adherence to another design guidepost known as the Open/Closed Principle -- should you make that available to other parties.

45

As is the nature of using interfaces, this decouples my users from being concerned with how I variously decide to refactor the implementations.

Let's take a step back and make sure we understand what the value of an interface is. It's not really what you've stated. Take the following example:

interface IFoo 
{
  void baz();
}

Now later you want to refactor this and realize the method shouldn't be named baz but bar instead. Can you do that refactoring without the users of the interfaces being concerned about it? No. How about adding or changing parameter types? No. You can add new methods to the interface without breaking things but that doesn't mean the user is not or should not be concerned. Nothing about this changes by using an interface or not defining one.

What about refactoring the implementation? You can (carefully) change the implementation of a public method on a concrete class without users knowing or having concerns about it. You can also change method's implementation to be incompatible with previous versions. Putting an interface in front of that doesn't really change anything with those two scenarios.

The only point of an interface is to define a contract which is independent of a specific concrete class. In other words, the client should not need to care which implementation it is dealing with. They are expected to be interchangeable with regard to the methods defined on the interface. It shouldn't even matter to the client whether the contract is defined on an interface or on a concrete class. All an interface does is decouple a set of public method signatures from a concrete class definition. It is a formal way to allow for substitution of implementations.

I think the C# Hungarian wart standard of prefixing interfaces with 'I' is misguided (and misguiding.) Interfaces don't do anything. There's no reason developers need to be concerned with whether the type they are working with is an interface. A lot of people might think this is just a preference but it creates a real issue. This standard means that you can't start with a class and then later decided it should be an interface without breaking everything that was using it. Therefore, C# APIs are so packed with interfaces that have a single implementation that I believe it has lead to a lot of developers who 'grow up' in C# not understanding the point of them.

In a nutshell, interfaces only support adding additional concrete implementations of a contract and especially allowing users to do so. And, naming standard aside, they can be used to do so without having to introduce them prematurely as long as you don't allow clients to bind to anything other than public methods. If you follow the 'I' wart standard, you must create an interface for every class you might ever want to later provide more implementations of without breaking user code.

27
  • 10
    I is misguided because I don't want to know if what I'm talking to is an interface or not. Without it, so long as you never use new on it, you never know exactly what it is. Sadly, while Java has avoided the I silliness they went and made it so that even when the source code doesn't know if you're talking to a concrete type the binary still knows. Sigh. Commented Sep 19, 2022 at 19:18
  • 14
    Absolutely agree. I've had to defend this position more than once and objectively, this preemptive creation of interfaces is the most damaging aspect, in my estimation. I look at C# source and I often find that every class has an interface and every interface maps to exactly one class. I don't want to pretend to be Dijkstra but 'cripples the mind' is on the tip of my tongue.
    – JimmyJames
    Commented Sep 19, 2022 at 20:09
  • 5
    I exists because the .NET framework treats class and interface very differently in terms of its inheritance model.
    – dan04
    Commented Sep 20, 2022 at 0:30
  • 7
    @dan04 According to one the founding members of the .NET team '... the "I" prefix on interfaces is a clear recognition of the influence of COM (and Java) on the .NET Framework. COM popularized, even institutionalized, the notation that interfaces begin with "I."' Which is quite a different reason than what you claim. If you didn't use the convention, the code will still compile and work just the same, right?
    – JimmyJames
    Commented Sep 20, 2022 at 13:46
  • 12
    @Voo To be clear, I am not advocating again interfaces. They are very useful for abstractions. What I think is detrimental is the pattern where every class gets an interface even if there's no practical purpose for it. If all you are doing with interfaces is stuff like this: IFoo foo = new Foo(); there's really no point to having the interface.
    – JimmyJames
    Commented Sep 20, 2022 at 16:22
11

In addition to the points already mentioned by JimmyJames, there's one additional drawback in exposing interfaces rather than classes: Your consumers can make their own classes implement them, e.g.

// consumer-side code
class MyFoo : IFoo
{
    void DoSomething() { ... }
}

Why is this a problem?

  1. Imagine you want to extend your IFoo interface with an additional DoSomethingElse method. You can't, without breaking MyFoo's code.

    (Note that C# 8+ allows you to work around this issue by adding a default interface method, but if you did not intend your interfaces to be implemented by consumer-side code, why clutter them with code that does not belong there?)

  2. Consumers can replace your implementation with theirs. Imagine that your API exposes a method FrobnicateFoo(IFoo foo), which is meant to be used with a Foo created by your factory methods. Well, your consumer can just pass in their own MyFoo instance instead.

    The general recommendation in C# is to make classes "sealed" unless they were explicitly designed to support inheritance. You can't "seal" your interfaces to consumer-side code.

What you can do is to only expose an abstract base class: If all its constructors are internal, your API users can't inherit from it. However, this is not something you need to do a priori: You can start with a regular class Foo. Later, should the need arise to use a subclass (example), you can make your factory method return a (private) SubclassOfFoo instead without breaking backwards compatibility.

16
  • 1
    @Simone: I wholeheartedly agree, that's the whole point of my answer: That OP's plan to hide the concrete Foo (Car) and only expose IFoo (Vehicle) to the API user might not be a good idea. :-)
    – Heinzi
    Commented Sep 21, 2022 at 9:42
  • 2
    Thanks for the feedback, good stuff. This brings up some tradeoffs I'm maybe dealing with, which is exactly what I was asking for. We're engineers, we expect any design to have its strengths and weaknesses, so it's a little disconcerting when I see one but not the other. Know thy enemy. That said, if these are what kind of issues I might be dealing with, I think I am ok in this case with the trades I am making, but I want to make sure I am understanding your points correctly, so let me try to respond to them and maybe you tell me where I'm off -
    – Bondolin
    Commented Sep 21, 2022 at 12:24
  • 1
    @Bondolin: Ad 1: Exactly. Ad 2: I agree, if you want to provide the option to have user-defined implementations of your interfaces, that's a great feature. You don't need it for easy unit testing though: You could have a public FrobnicateFoo(Foo foo) which calls an internal FrobnicateFoo(IFoo foo), and the internal method is visible to your unit test project via the InternalsVisibleTo attribute. This would allow you to hide your IFoo interface, so that any changes to it would only affect your project and your unit test project, not the public API.
    – Heinzi
    Commented Sep 21, 2022 at 14:04
  • 1
    #1 is not a drawback. If you change the definition of IFoo, you should break consumer code, because it no longer works with your library. This is why interfaces take so much thought and should only be changed when necessary, and why the Interface Substitution Principle is so important. Commented Sep 21, 2022 at 22:30
  • 2
    #2 is not a drawback either; in fact it's the whole point. If you write the method FrobnicateFoo(IFoo foo) and it doesn't work with every valid implementation of IFoo, then your method is wrong. Why is this supposed to be a downside? The answer is to fix your method signature, or fix your method. Honestly it sounds to me like you are using and understanding interfaces very incorrectly. Commented Sep 21, 2022 at 22:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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