13

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?

UPDATE: 3 years later, I'll share my experience.

(since people are still finding this thread and finding it useful judging the votes and 5K views)

Learning unit-testing was hard, but I got to appreciate the benefits in these cases:

  • Writing a recurring-billing system where 100x different scenarios can happen. How the heck would you test that manually!??

  • Writing and refactoring a web API client, where the server can change or break things without notice, or introduce unexpected behaviors for some classes

I personally don't think interfaces are a code-smell, neither is Moq; but got to find that #region is a code smell so I keep one file per class. I do not like to pass concrete classes via Dependency Injection, so except in rare cases, I use interfaces for anything sent via Dependency Injection.

new keyword? It's fine for deterministic classes, but anything nondeterministic should be injected via Dependency Injection to make the class deterministic. As an example: Time. Do not use DateTime.Now, but instead, create IDateTimeService that returns the time. If creating a billing system, you'll then be able to test scenarios at specific times in a deterministic fashion.

I also had issues for showing dialogs with the creation of the ViewModel. What if the dialog's ViewModel requires IDateTimeService to show the time? I solved the problem in HanumanInstitute.MvvmDialogs by adding IDialogService.CreateViewModel method that uses a function injected in its constructor (often set to () => Locator.GetService<T>() or a custom method for tests.

Creating interfaces is actually quite easy, and helps me see what a specific class is really doing for the public and how to use it. When I get to use my carefully-crafted service, I don't care about its internal implementations. To avoid duplicate comments, /// <inheritdoc /> is your friend!

I create interfaces for most business classes, not for models. As for ViewModels, it's generally not necessary unless the ViewModel is used by another ViewModel.

Unit test declarations

These are tips I developed that I've never seen anywhere so I'm sharing it for those interested. Dependency trees can get complicated in some cases, and you often don't need to initialize everything for each test. I initialize each object in a lazy way when needed, and don't need to care about order and dependencies. The objects are available when needed.

  • The main class I'm testing is available as Model (not sure whether it's the best name but it doesn't really matter as long as it's consistent).

  • When creating the test class, I then decide which concrete dependencies I want, and which ones I want to mock.

  • In this case, I have an audio encoder. I decided to split the code in 3 levels: AudioEncoder for the raw encoding API, EncoderService to take a list of files and pass them through AudioEncoder with progress report and everything without caring about UI, and MainViewModel that allows configuring EncoderService (adding files and folders), and displaying the data and settings in a way that is convenient for the UI.

  • Here, I'm using concrete MainViewModel, concrete EncoderService, and mock AudioEncoder. AudioEncoder has separate Integration tests. DialogService is concrete, and contains a DialogManager that is mocked.

  • For easy initialization of Mocks, you cannot use lambda expression on the right-side of ??=, so I created an Init method that makes the problem go away! Poof magic! See MockAppPaths. I also often create methods to set test-specific mock configurations. Note that if you use Moq Linq syntax to declare your mock quickly, it calls SetupAllProperties, which causes CallBase to not work. You generally want to avoid this.

  • FakeFileSystem has a standard implementation in a shared test library. All members are virtual so I can still mock it to override methods as needed.

public MainViewModel Model => _model ??=
    new MainViewModel(Encoder, DialogService, FakeFileSystem, MockAppPath.Object);
private MainViewModel _model;

public IEncoderService Encoder =>
    _encoder ??= new EncoderService(FakeFileSystem, DialogService, MockAudioEncoder.Object, new FakeDispatcher());
private IEncoderService _encoder;

public Mock<IAudioEncoder> MockAudioEncoder => _mockAudioEncoder ??= new Mock<IAudioEncoder>();
private Mock<IAudioEncoder> _mockAudioEncoder;

protected T Init<T>(Func<T> func) => func();

public Mock<IAppPathService> MockAppPath => _mockAppPath ??= Init(() =>
{
    var mock = new Mock<IAppPathService>();
    mock.Setup(x => x.AudioExtensions).Returns(new[] { "mp3" });
    return mock;
});
private Mock<IAppPathService> _mockAppPath;

public Mock<FakeFileSystemService> FakeFileSystem => _mockFileSystem ??= new Mock<FakeFileSystemService>() { CallBase = true };
private Mock<FakeFileSystemService>? _mockFileSystem;

public IDialogService DialogService => _dialogService ??= new DialogService(MockDialogManager.Object);
private IDialogService _dialogService;

public Mock<IDialogManager> MockDialogManager => _mockDialogManager ??= new Mock<IDialogManager>();
private Mock<IDialogManager> _mockDialogManager;

To make things even easier, I got this base class.

/// <summary>
/// Base class for test classes.
/// </summary>
public class TestsBase
{
    protected readonly ITestOutputHelper _output;

    public TestsBase(ITestOutputHelper output) => _output = output;

    /// <summary>
    /// Allows using a lambda expression after ??= operator.
    /// </summary>
    protected T Init<T>(Func<T> func) => func();
}

public class EncoderServiceTests : TestsBase
{
    public EncoderServiceTests(ITestOutputHelper output) : base(output) 
    { }

    // ...
}

With these guidelines, testing is becoming pretty smooth and orderly!

I hope this helps someone.

0

5 Answers 5

11

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

10
  • 1
    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. Feb 19, 2019 at 16:35
  • 4
    @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, 2019 at 16:41
  • Suppose you need to test an application that works with a database via SqlQuery object. This object literally accepts SQL strings. So to UT this one would need to parse these strings in UTs. Isn't it better to create interfaces for providers that use this query object and mock them?
    – Yola
    Feb 11, 2021 at 0:52
  • "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." Au contraire. When an interface makes a breaking change (i.e. not just adding a new member but adjusting an existing member), you should re-evaluate all tests that rely on it.
    – Flater
    Jun 29 at 8:18
  • "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." You're moving the goalposts without answering the question. Those "units of code" still imply that your codebase is made up out of (at least) more than one unit of code, which leads to the same end goal: needing to mock a depended on unit in another unit's tests. Whether you're talking about single classes or small groups of classes is irrelevant as to how/whether to mock dependencies in a unit test.
    – Flater
    Jun 29 at 8:20
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.

3
  • 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. Feb 20, 2019 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? Feb 20, 2019 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. Feb 26, 2019 at 3:40
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.

6
  • 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. Feb 20, 2019 at 9:41
  • 2
    "*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, 2019 at 11:05
  • 4
    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, 2019 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 Feb 20, 2019 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 Feb 20, 2019 at 12:41
2

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.

2
  • 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." Feb 19, 2019 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. Feb 19, 2019 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.