4

Should I define an interface for every public behavior class (excluding data classes)?

I've spent many hours searching and reading to find a clear answer. If I search "Do you define an interface for every public class", 90% of the answers say you should not. However, I have yet to find any such comment explain how to do unit testing otherwise.

Some say Moq can mock concrete classes, but only for members that are declared virtual, and since the constructors cannot be marked virtual, there is no way for Moq to prevent code from running in the constructors (AFAIK). I have no interest in marking every member of every class as virtual.

It seems the answers to this question fall into 3 categories:

  • those who test everything
  • those who test only part of the code
  • those who don't bother with unit testing

I've seen all the arguments on both sides already, but I still haven't seen any other good way of doing extensive unit testing of the code. Why then, are 90% of the people advocating against this?

The way I'm doing it for now is to place the interface at the top of the same file within a #region, so there is no increase in files, I can easily navigate to the implementation, and it doesn't clutter the code view. If some interface needs to be implemented several times, nothing prevents me from moving it into separate files later.

One of the main reasons for creating such interface is because of limitations of mocking frameworks. Let's say the next version of .NET allowed mocking frameworks to mock non-virtual methods, should I still create those interfaces?

Taking a simple example, I have class A, B and C. A depends on IB and IC for testing. Even if not needed for mocking, A still needs instances of B and C injected via dependency injection. Using interfaces is optional for dependency injection but I have yet to see good examples recommending to not use interfaces. So in this hypothetical scenario of not being needed for mocking, should I still create those interfaces or not?

And finally, if creating such interfaces is a good approach (which many disagree with), is there any tool that can auto-generate those interfaces at compile-time so I don't have to copy the method signatures and comments all the time?

7

Should I define an interface for every public behavior class (excluding data classes)?

Not necessarily.

I've spent many hours searching and reading to find a clear answer. If I search "Do you define an interface for every public class", 90% of the answers say you should not. However, I have yet to find any such comment explain how to do unit testing otherwise.

Short answer: you are allowed to "unit test" concrete dependencies.

Longer answer: separating interface from implementation allows you to decouple the two. Another way of expressing the same idea is that the interface provides a seam that can be used to introduce alternative implementations.

In the language of Parnas, the interface gives us to insulate modules from the decision of how a particular capability should be implemented.

We tend to use substitutes often in our tests, particularly in our developer tests, because predictable behavior depends on being decoupled from shared mutable state, and fast execution requires avoiding slow code.

But if a particular component is already isolated from shared mutable state, and doesn't include slow code, the argument for not just using it in your test becomes a lot weaker.

If you apply the Dependency Inversion Principle, an interesting thing happens -- the definitions of the interfaces move to the modules that are going to use them, rather than living in the modules that provide the implementations.

In other words, we have the possibility of a substitute not because our implementation supports an interface, but because the consumer does.

class Example {
    void fizz (SomeInterface x) {...}
    void buzz (SomeImplementation x) {...}
}

It's Example, the consumer of the implementation, that gets to decide how flexible its needs really are.

See Dependency Injection Is NOT The Same As The Dependency Inversion Principle for more details.

So if you find yourself writing a test, and wishing that you could more easily replace some concrete implementation with an alternative, that's a code smell telling you that the interface of your test subject is too tightly coupled to the implementations of the collaborators. So go fix that first....

Also, I saw several people say you should never use the "new" keyword in the code

That's not quite right. If you are asking the runtime to store data on the heap, then you are going to be invoking new somewhere. What they are suggesting is that you not mix the new operator into your logic; the decision of how we instantiate an instance of an object can be moved to a module of its own (aka, a "factory").

Should it be moved? Maybe; there are two pressures here -- if the instantiation is likely to change during the lifetime of your code, then having a separate module limits the impact of the change. If you are likely to want to replace the implementation as you move from one environment to another (ex: test vs production), then that's a good reason to have a replaceable module separating that code.

but don't specify whether that applies for classes like DateTime, TimeSpan or StringBuilder

Right. My answer is that if we have a stable dependency that gets lots of eyeballs to help share the burden of catching bugs, that doesn't introduce an indirect dependency on mutable state or the imperative shell, and isn't something that we want to vary from one environment to another, then introducing a bunch of ceremony to isolate our "unit" from this dependency is a waste of time and annoys the pig.

So StringBuilder goes into the same bucket as String goes into the same bucket as array, as in "of course you can just use it". If we later need to replace String with a Rope in some contexts -- fine, I'll pay off to that.

Should I use Dependency Injection between those classes?

Horses for Courses -- in some cases, it can make sense to pass in your dependencies as replaceble arguments even when the expected implementation is isolated. I'm not going to reject a pull request just because you didn't make the decision I would have given my limited understanding of the problem that you were investigating -- but I might ask you to include a note explaining the motivation for your choice.

  • Also, I saw several people say you should never use the "new" keyword in the code -- but don't specify whether that applies for classes like DateTime, TimeSpan or StringBuilder. Then on the other side are people saying extracting interfaces isn't always necessary, but then using those classes means I'll be using "new" in the code to create it. Or in those cases, do they still use the IOC to create the class instances and inject it even if not using interface? Using Dependency Injection without interface feels weird to me, it doesn't remove any coupling at all. – Etienne Charland Feb 20 at 2:31
  • +1 for "If a particular component is already isolated from shared mutable state, and doesn't include slow code, the argument for not just using it in your test becomes a lot weaker." Had to read that a few times to get it. So if I close the external interaction boundaries with mocks, series of classes can work together without risk. Should I use Dependency Injection between those classes? In some cases it's good to do Dependency Inversion (APIs between modules) but in most cases simple Injection is all that's needed unless I'm mistaken? – Etienne Charland Feb 20 at 2:52
  • @EtienneCharland new has to exist somewhere. But anything that contains new Foo() KNOWS that Foo is an implementation and not an abstraction. Spread that knowledge around and it becomes very hard to turn Foo into an abstraction later when you need to. Keep only a few things, like main() and factories, knowing which one Foo really is and you don't have to touch anything else when you do change it. This is the same issue if you switch to a keyword interface or an abstract class. In fact, the only difference is only one choice supports multiple inheritance. But using code shouldn't care. – candied_orange Feb 26 at 3:40
6

The underlying problem is the need for mocking in unit testing. Mocking is a code smell - it is unavoidable in some cases, but should not be pervasive in your unit tests.

Mocking is necessary for classes which are interfaces to external services, perform input/output, or are non-deterministic (time, random number generation). But you shouldn't need to mock every single class in your code. Doing this will lead to brittle tests, where every change in an interface will force you to update the corresponding tests mocks, which defeats the purpose of unit testing.

Mocking frameworks like Moq should IMHO only be used in the case of legacy codebases where you need to add tests to existing code which was not designed for testing. It should not be necessary in well-designed code.

To address the concern that a unit test should only test a single class: This is a misunderstanding. A unit test should test a unit of code which can be tested independently. This might be single function or a set of classes depending on how tightly coupled the code is. For example Math.Max() has no dependencies or side effects, so can be tested independently with no need for mocking. But we don't actually care if Max internally uses some helper classes. This is an implementation detail, as long as the public behavior corresponds to the specified requirements.

Ideally you want as much as your code to work like Max: A well defined interface with no public dependencies or side-effects. This have advantages in two ways:

  • You don't need any form of mocking to test Max.
  • If some other component depends on Max there is no need to mock it.

(On that token, let me dispel the myth that static methods are bad for testing. As Math.Max() shows, a static method without side-effects is actually the best thing ever for testing. Just avoid static state.)

Of course I/O and side-effects are necessary in any code. But you want to separate this from any complex logic which requires unit-testing.

I have class A, B and C. A depends on IB and IC for testing. Even if not needed for mocking, A still needs instances of B and C injected via dependency injection.

The problem with A B C examples is the answer depends on what A, B and C actually is. So lets take some examples:

A is InvoiceGenerator, B is Customer and C is an Order. Easy - you instantiate Customer and Order with data appropriate for the test. No need for mocking.

But if say A is InvoiceGeneratorAndSender and B is SmtpServer then you may temporarily need to mock SmtpServer, but it would be better to separate the invoice generation from the mailing. Invoice generation could then be tested without any need for mocking. The mailing would be a really shallow wrapper over the SmtpServer so wouldn't need to be unit tested at all. (But should be integration-tested.)

  • What you're describing are integration tests, where we tests whether several classes work properly together, not unit tests, which are run against a single class. – Etienne Charland Feb 19 at 16:35
  • 3
    @EtienneCharland: No, integration tests are tests which affect multiple subsystems. See for example: martinfowler.com/bliki/IntegrationTest.html A unit test can involve multiple classes but should not touch external services or input/output. – JacquesB Feb 19 at 16:41
4

Should I define an interface for every public behavior class (excluding data classes)?

You should stop thinking this way.

Do not decide you have a Foo and then decide you need an IFoo interface.

Stop thinking about what Foo is and think about what uses Foo, how it uses it and how it doesn't.

Interfaces, abstract classes, compositional delegators, and any other form of abstraction should not be designed around what something is. That should be designed around how something is used.

Most importantly, whatever is doing the using shouldn't know which form the abstraction takes. It shouldn't know if Foo is a keyword interface, abstract class, or delegating to composition.

That not knowing frees you to do it however you like. It lets you change your mind. It keeps you from breaking everything. It makes your question a pointless one because if you do this the answer to your question doesn't matter.

This is true if you always unit test, sometimes unit test, or never unit test. This isn't about unit testing. This is about protecting things from knowing more than they need to. Do this and you'll stop feeling like you have to establish some interface convention.

  • For now I'm refactoring a small stand-alone class library to practice all these concepts. What you're describing sounds like public vs private/internal members and classes. Is there something more to it? It sounds like you're recommending to create interfaces only on code boundaries where the library is used from the outside -- but by doing so I have to unit test blocks of classes instead of individual classes. – Etienne Charland Feb 20 at 9:41
  • "*It sounds like you're recommending to create interfaces only on code boundaries where the library is used from the outside -- but by doing so I have to unit test blocks of classes instead of individual classes. *". Spot on. You should only write tests for those boundary APIs; never try to write tests for the inner workings. They are implementation details that should be free to change in any way as long as they don't affect the boundary APIs. Trying to test such freely changing code will lead to brittle tests or restrictions on how you change. Neither is good. – David Arno Feb 20 at 11:05
  • 3
    And don't get hung up on the term "unit test", just write good quality, useful automated tests and leave the semantics to others. – David Arno Feb 20 at 11:06
  • Could you take a look at this code I refactored? Small class library. Currently, all classes have an interface. Truth is, most classes can be used from the outside at some point; with the exception of FFmpegParser which could be left as a static class. Is there anything you would change to the project structure or interfaces? github.com/mysteryx93/FFmpeg.NET/tree/master/FFmpeg – Etienne Charland Feb 20 at 11:11
  • I understand what you're saying, conceptually. In practice, however, for the library I'm working on right now, nearly all the code is part of the API, since I expose usage at 3 levels (working directly with the process, easy calling with preset args such as muxing and encoding, and accessing these presets via static methods for extra convenience). On top of that, 2 of the features are mostly only used by the calling application: UserInterfaceBase and TimeLeftCalculator. This leaves nothing as non-public interface. Perhaps I should leave it like that and revisit what you say for other code – Etienne Charland Feb 20 at 12:41
1

Part of the reason why people argue against it is as a cost/benefit analysis. If you are working in a well-established code base, it may be that you have high confidence that your system works (it has been used by customers for 5 years) but poor unit test coverage. In this case you need to weigh up the time taken to not only to create the unit tests but to refactor the code to be easily unit testable, versus the benefits of doing so. In an established code base such as this one of the main reasons for unit tests is for regression testing and to aid in refactoring. If you find you have to refactor a working but poorly tested code base to do this, you might be creating a headache for yourself.

Again, this depends on a very great many factors. Frequently changed code requires far more extensive regression tests than code that you know from experience has been untouched for ten years.

In a green field, brand new, professional project I'd always very highly recommend extensively unit testing everything. But as legacy systems are usually maintained to help a company turn a profit, you need to consider carefully if adding unit tests will help or hinder you to achieve that goal.

  • Here I'm talking strictly from the perspective of writing new code. How to deal with existing code-base (often not structures for unit testing nor dependency injection) is a whole other concern. I am however refactoring a whole application because it grew into way too much spaghetti and I need to update my skills with the latest practices to grow it any further. In a business environment, however, the best advice is usually "if it's not broken, don't fix it." – Etienne Charland Feb 19 at 16:14
  • As for the cost/benefit, the time it is taking me to write unit tests is frustrating, but from what I read, it is common for unit testing to take just as much time as it takes to write the code itself (or half for those more efficient). Many have also stated that overall, it shortened the time to complete the project. Considering we always need to plan 4-10x the development time for testing and debugging, I can believe that. I also read about a research from Microsoft where those using unit testing took 35% more time but had 50% less bugs -- resulting in BIG time savings later on. – Etienne Charland Feb 19 at 16:38
0

This needs a two-fold answer:

  • It is a good idea to decouple your code as it will help you while developing and while testing

  • Having interfaces that copy the implementation is a smell since you are not design the real abstraction that lies behind that). Having Car and ICar is a smell since it does not give you any explanation of your business: you might have the interface Car and implementation like Turism, SUV, PickupTruck... (Sometimes, this is also done with inheritance as another way of abstraction)

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.