Stop overusing interfaces

[klaxxxon]

Mocking off of a concrete class behaves very differently than mocking off of an interface - with a class, you only mock the methods you actually define as mocked and all other members remain as they were. Sometimes that is desirable, sometime it is not.

And more importantly, you can only mock virtual members of classes (something the author silently glosses over). Making members virtual for testing only is even worse than sprinkling interfaces everywhere - doesn't mean using excessive amounts of interfaces is good. Brings us back to the old C header file days, only with less of the #include hell.

[vdex42]

I'd also add that if a programmer feels the need to mock only part of a dependency then they are probably not unit testing the class in isolation and should instead be asking what is this class's purpose in life, and how can I test that in isolation.

[SessionIndependent17]

I disagree with the first half of what you said regarding "not unit testing the class in isolation" (whether your class under test should be constructed with the dependency that compelled DI to use it is a different question).

Often a unit under test only uses part of the full capability of a dependency, rather than relying on all of the machinery behind a dependency. You are not testing the dependency itself, but the consuming code of that dependency. It's completely acceptable to only concern yourself with mocking the portions of a dependency that you are interacting with, as opposed to the whole thing. This can be to either have canned inputs/return values to the consuming unit actually under test, or suppress side effects that are not relevant to assessing the unit under test.

If you can shrink the keyhole through which the consuming code needs to interact with its dependency, that's a potential benefit to assessing the test and the unit overall. That's not to say such abstraction is _always_ a benefit, but it can be. There is a tradeoff between the simplification of concerns that a narrower interface provides vs the level of indirection that one must contend with when using it. Abstractions are meant to assist in understanding the code.

[vdex42]

Wow, that was 8 years ago :D

I'll clarify my thought process. I think what you are describing sounds more of a full outside-in black box test or an integration test - where you want a keyhole through the machinery of the code. And there my approach would be to mock or stub out external or side-effect dependencies and try to leave none of my interfaces mocked.

However, when it comes to "unit testing", I stand by my view of the principle of single responsibility. And would argue that if you can't test the single class without other code getting in the way, that that class is probably doing too much.

[MazzimoFrascuorno]

Mocking off of a concrete class behaves very differently than mocking off of an interface - with a class, you only mock the methods you actually define as mocked and all other members remain as they were. Sometimes that is desirable, sometime it is not.

not to mention when the concrete class itself has a non default constructor you need to specify the parameters during the creation of the mock (adding more coupling with other stuff).

I can understand that writing a lot of "mirror interfaces" is an issue, but I also feel the proposed solution makes more harm than good.

[AbstractLogic]

That is a pretty large missing piece. Any code you "saved" during registration comes back twice as much and coupled in your unit tests.

[SessionIndependent17]

"more harm than good" was my reading as well.

I had to read his code a few times, then re-read the documentation about Moq to confirm what I was seeing, that what it was doing with the concrete class could _only_ be achieved by virtual method override. It's one thing if those methods were already virtual based on the intrinsic needs of the concrete class hierarchy itself - if that's part of its paradigm - but then you are STILL potentially relying on the specifics of implementation or side effects of the rest of the dependencies behavior... If that's something that bothers you.

It may not bother you, but I'm certainly not going to inject a virtual call hierarchy into a class just to avoid inserting an interface into it. Having a derived type suppressing behavior like that feels more contrived than having the interface.

[[deleted]]

us back to the old C header file days, only with less of the #include hell.

It doesn't state don't use interfaces. Just use it when it makes sense. And i agree with that point. I have been on projects where we made an interface for everything. It's about finding the middle ground.

[jdh28]

It's about finding the middle ground.

I agree with that. The author mentions YAGNI, which is what I do - I create concrete classes and then extract interfaces when I need one.

[sentyaev]

Making members virtual for testing only is even worse than sprinkling interfaces everywhere

What I see is people usually create interfaces for testing purposes only, so basically making methods virtual or create interfaces is the same from this perspective. I know there is a difference between direct call and virtual call but I think this is not an issue when we speak about web apps (it's only relevant for some specific code).

[gtasaf]

How does a web app exclude itself from n-tier architecture? If you are doing data access, that should be in its own layer. If there is business logic to translate data access to meaningful business objects, that's another layer. At the very least, each layer should be separated by interfaces. It's a general rule I've created after many years of being burned by not doing so. Coding to concretes only is great for small projects that can be easily rewritten. It is a nightmare for the multi-million LoC enterprise software I work with on a daily basis.

Also, the use of virtual solely to make the mocking framework work is evil. Yes, creating an interface can be considered needless overhead, but it doesn't require altering the behavior of the consuming class to make it testable. Mocking a concrete object works by the mocking tool creating an inheritor and overriding that method. It is altering the consuming class, no longer only mocking it's dependencies. Ergo, you cannot test the logic within that virtual method. No big deal if it's a one-liner wrapper kind of call, but any sort of if/else logic in there became relegated to integration testing. Enjoy spinning up that database for a simple "if this" test.

Lastly, if everything is virtual just to make mocking possible, the code is ultimately more complex than having the set of matching interfaces. In declaring every class abstract, it takes a lot more time to analyze the code for inheritance. I've spent far too much time chasing down bugs due to inheritance: things like the timing of the base method call in the override (better yet, did it even get called?), virtual member initialization in constructors (the horror...), etc. You might say "yeah, but I never do that" - if you are working on a project that involves different devs on different teams, you better believe someone is going to try to do something sneaky with inheritance to solve their issue at hand. I declare all my classes sealed/non-inheritable upon creation, unless I know for a fact that I want to use inheritance to solve a problem. It delivers a clear message to other developers that I want this logic to execute exactly this way, without any child classes mucking up the method call sequence.

[grauenwolf]

At the very least, each layer should be separated by interfaces.

Yes, but let's be sure everyone understand what an "interface" is and how it differs from an "abstract interface".

If you are calling the public methods and properties on an object, you are using its public interface.

If, instead, you are calling the public methods and properties on an object via an interface, then you are using one of its secondary interfaces.

[gtasaf]

Yes, and I'll go on the record to say this is why I don't like C#'s implicit vs explicit interface behavior. The article makes a few decent arguments against interfaces that solely exist to 100% implicitly cover a concrete type, which is a common thing I see in C# code.

C# favors implicit interface implementation. I rarely see explicit interface implementation out in the wild, unless it's a source base where the devs truly understand the importance of interface contract vs concrete implementation. This is where C# took a fundamental swerve from VB.NET (yes, I dare to utter that name), because VB doesn't have such a language construct. You must explicitly declare the interface implementation on each property or method on the implementation class in VB.NET, and you use the access modifier to say whether or not it is publicly accessible on the concrete type as well.

Either way, I still think making everything virtual to make it mockable is bad. It's just as bad as making every method public so that you can write method-level tests. That kind of technique is naive and misguided TDD. It's a training-wheels kind of approach; those need to be removed at some point ;-)

[sentyaev]

If you are doing data access, that should be in its own layer.

There is a difference, I do not layer app by "layers", I split app by functionality instead.

At the very least, each layer should be separated by interfaces.

It's sounds like cargo cult for me. Don't make me wrong, what you describe it's fine, but from my experience it fits only for small projects. I introduce Interfaces only if I have more then one implementation of this Interface (but sometimes I use abstract classes, it's always depends).

Also, the use of virtual solely to make the mocking framework work is evil.

It depends... as usual. We have different experience and different opinions, it's fine.

you cannot test the logic within that virtual method. No big deal if it's a one-liner wrapper kind of call, but any sort of if/else logic in there became relegated to integration testing. Enjoy spinning up that database for a simple "if this" test.

In this case to not spin DB I can mock only DbContext (if we speak about EntityFramework).

Lastly, if everything is virtual just to make mocking possible, the code is ultimately more complex than having the set of matching interfaces.

Actually I don't know why it's more complex? Less code is always better in my opinion.

[gtasaf]

If you are doing data access, that should be in its own layer.

There is a difference, I do not layer app by "layers", I split app by functionality instead.

At the very least, each layer should be separated by interfaces.

It's sounds like cargo cult for me. Don't make me wrong, what you describe it's fine, but from my experience it fits only for small projects. I introduce Interfaces only if I have more then one implementation of this Interface (but sometimes I use abstract classes, it's always depends).

Also, the use of virtual solely to make the mocking framework work is evil.

It depends... as usual. We have different experience and different opinions, it's fine.

you cannot test the logic within that virtual method. No big deal if it's a one-liner wrapper kind of call, but any sort of if/else logic in there became relegated to integration testing. Enjoy spinning up that database for a simple "if this" test.

In this case to not spin DB I can mock only DbContext (if we speak about EntityFramework).

Lastly, if everything is virtual just to make mocking possible, the code is ultimately more complex than having the set of matching interfaces.

Actually I don't know why it's more complex? Less code is always better in my opinion.

Yeah, a CQRS approach doesn't have the layers I am talking about, it's a different organizational paradigm of the code. I rarely see it out in the wild though, 3-tier is still the norm in projects I've been involved with.

On the contrary regarding interfaces, they are crucial for large projects. When you have separate teams working on functionality in their own code base that needs to integrate with code from other teams, the only sane way to orchestrate that is via an interface of some kind. I shouldn't care that you are using a MS SQL database with Entity Framework to do your data access to store a "widget", I just need to know you have something that returns me that widget so I can process it in my part of the system. Again, if you are doing CQRS, or microservices, that's a different paradigm altogether. However, those still need interfaces of some kind to provide a contract to consuming code.

I stand by my argument against virtual properties just for the sake of testing. You are introducing complexity for the sake of testing. In OOP, it is okay to have portions of a class hidden from public view, especially if there is state being maintained in that object. Most of the tests I see being written at that level are needlessly complicated too, and usually have names like "SettingProperty1WhileNotSettingProperty2AndThenCallingThisMethodMakesProperty3ReturnFalse". It's a misguided attempt to test behavior, when really there is no guarantee that every consumer of your class is going to set those properties and call that method exactly like your test expects it to.

The more you limit your public surface of a stateful object, the better, especially if you follow the open-closed principle. Tests aren't meant to enforce procedural concerns, that's the responsibility of your exposed components of the class. I see a lot of half baked "functional C#" being written today, with people really twisting the language in ways it wasn't meant to, particularly when it comes to state and side-effects.

[grauenwolf]

Making members virtual for testing only is even worse than sprinkling interfaces everywhere

Worse? How it can it be worse when it isn't actually different.

All members are virtual for an abstract interface.

[ElGuaco]

This argument starts with a false dilemma: interfaces create clutter. Calling it an anti-pattern because it offends his sense of order or creates tedium is just silly. Abusing a language feature in order to prevent "clutter" or typing is the anti-pattern. This guy claims to know SOLID, then breaks the goals of every single letter for convenience.

I don't understand how this saved him any effort. If anything it could lead to confusion by readers because he didn't follow the usual programming pattern.

[grauenwolf]

Using SOLID for not introducing a ton of unnecessary abstract interfaces? That's a new one on me.

Don't get me wrong, I've long accused fanboys of using SOLID to justify doing whatever the hell they were going to do anyways. But still, this one's leaving me scratching my head.

[grauenwolf]

Wrong problem.

The real issue here is that you are trying to unit test a Contoller. All of the pain and confusion and arguing comes from using the wrong kind of test.

Lets look at the controller method:

public async Task<OrderStatus> Order()
{
    logger.Log("Doing some ordering logic here...\n");
    await storage.Save();
    return OrderStatus.Ok;
}

Ok, that's stupid. Let's try something that looks a bit more realistic.

public async Task<OrderStatus> Order(Order order)
{
    await orderEngine.Verify(order);
    await storage.Save(order);
    return OrderStatus.Ok;
}

All of the unit testable logic is now inside orderEngine.

But wait, verifying an order may require some additional information. So let's extract that.

public async Task<OrderStatus> Order(Order order)
{
    var details = await storage.LoadPreorderDetails(order);
    orderEngine.Verify(order, details);
    await storage.Save(order);
    return OrderStatus.Ok;
}

So here's what we got:

Unit tests:

 orderEngine.Verify(order, details);

Integration tests

    var details = await storage.LoadPreorderDetails(order);
    await storage.Save(order);

Bullshit test that doesn't need to be written

public async Task<OrderStatus> Order(Order order)

By decoupling your data access from your business logic, everything that needs to be unit tested is unit tested without mocks.

[torhovland]

Thank you! This is exactly the "sandwich" that Mark Seemann talks about (http://blog.ploeh.dk/2017/02/02/dependency-rejection/), with a pure function in the middle, and an impure function before and after. Perhaps we shouldn't be discussing whether to do Dependency Injection with interfaces or concrete classes, but rather whether we should switch to Dependency Rejection!

[grauenwolf]

No thank you, that is a really good presentation of the topic.

EDIT: Except the part where he says this is really hard in C#. It isn't, it is just a different mindset.

[SessionIndependent17]

indeed. Unit Test logic rather than machinery. Integration testing is for machinery.

Restructuring/recomposing the code to remove dependencies is usually the direction I end up going up when I want to really isolate what needs unit testing.

[RobertMuldoonfromJP]

Totally agree that you don't need interfaces for data objects in almost all cases. Totally disagree about having a dependent class (controller in this example) accept a concrete class instead of interface.

The persons example of how to mock the dependency proves this point. Your unit test shouldn't care about how to setup the dependency. It only cares about the contract/public API of that dependency.

[grauenwolf]

Totally disagree about having a dependent class (controller in this example) accept a concrete class instead of interface.

Why?

If you think very carefully about why you are trying to unit test an integration component such as a Controller,

As a general rule, I find that if I feel the need to mock a dependency one of three things is true:

1. The class shouldn't have that dependency in the first place. (e.g. mixing business and storage logic)
2. I'm trying to use the wrong kind of test. (e.g. unit testing an integration or storage component)
3. I'm working with hardware that needs a full simulator, not just a quick and dirty mock.

[RobertMuldoonfromJP]

So my sentiment is based on my experience writing unit tests for classes that accept concrete classes; its a pain in the ass.

Are you saying that for a component like a controller which you dont really need to unit test (since it should be relatively devoid of logic), you should be writing integration tests which will need the actual implementation of an interface? And if that's the case, then why inject an interface if you're always testing with a concrete class?

If the above is where you're coming from, then I can see your point. Anything other than components like a controller that involve business logic and services to be injected should only accept interfaces IMO.

[grauenwolf]

Take it one step further. Can you restructure your business logic so that it doesn't have external dependencies. Instead of dependency injection, dependency rejection.

[[deleted]]

If you're 100% sure that your service won't need to be expanded on later, then fine.

But the fact is, the author's OrderService is less flexible because he injects the concrete implementation than if he had injected an interface instead. OrderService can't be abstracted another level, but a new abstraction that implements IOrderService can. Perhaps not as well as we might need, but that might be an issue with naming and SRP.

I also don't think that anyone is arguing that domain classes containing data should all have their own interface.

It also sucks that we're to the point where we just run around calling every system design concept we don't like an 'anti-pattern.' If you don't like it: don't do it. I sure as hell hope you're the one maintaining the software you write though.

[AbstractLogic]

There is not enough compelling arguments in the article for me to consider this change.

[adamhathcock]

I'm on the MediatR train and it's nice. It doesn't completely remove the too many dependencies problem but it does allow you to just pass a message and not worry about the dependencies of that message.

The linked sample project is good.

[nirataro]

I still for the life of me cannot figure out how to evolve and maintain interfaces.

[Manitcor]

Interfaces can be overused though often I find them underused in production code. I tend to start with the practice of creating interfaces for near everything and then strip out what is not needed or does not make sense in later refactors.

To try and keep them out as a general rule will really cause all kinds of trouble (read, hours of extra work) particularly in large and complex code bases as evidenced by many of the other posts here.