19

I suspect I've made a schoolboy error here, and am looking for clarification. A lot of the classes in my solution (C#) - dare I say the majority - I've ended up writing a corresponding interface for. E.g. an "ICalculator" interface and a "Calculator" class that implements it, even though I'm never likely to replace that calculator with a different implementation. Also, most of these classes reside in the same project as their dependencies - they really only need to be internal, but have ended up being public as a side-effect of implementing their respective interfaces.

I think this practice of creating interfaces for everything stemmed from a few falsehoods:-

1) I originally thought that an interface was necessary to create unit test mocks (I'm using Moq), but I've since discovered that a class can be mocked if its members are virtual, and it has a parameterless constructor (correct me if I'm wrong).

2) I originally thought an interface was necessary to register a class with the IoC framework (Castle Windsor), e.g.

Container.Register(Component.For<ICalculator>().ImplementedBy<Calculator>()...

when in fact I could just register the concrete type against itself:

Container.Register(Component.For<Calculator>().ImplementedBy<Calculator>()...

3) Using interfaces, e.g. constructor parameters for dependency injection, results in "loose coupling".

So have I gone mad with interfaces?! I'm aware of the scenarios where you would "normally" use an interface, e.g. exposing a public API, or for things like "pluggable" functionality. My solution has a small number of classes that fit such use cases, but I wonder if all the other interfaces are unnecessary, and should be removed? Regarding point 3) above, won't I be violating "loose coupling" if I was to do this?

Edit:- I'm just having a play with Moq, and it seems to require methods to be public and virtual, and have a public parameterless constructor, in order to be able to mock them. So it looks like I can't have internal classes then?

6
  • I'm pretty sure C# doesn't require you to make a class public in order to implement an interface, even if the interface is public... Commented Jul 10, 2013 at 10:28
  • 1
    You're not supposed to test private members. This can be seen as an indicator of the god class anti-pattern. If you feel the need to unit test private functionality then it's usually a good idea to encapsulate that functionality into it's own class.
    – Spoike
    Commented Jul 10, 2013 at 11:44
  • @Spoike the project in question is a UI project, where it seemed natural to make all the classes internal, but they still need unit testing? I've read elsewhere about not needing to test internal methods, but I've never understood the reasons why. Commented Jul 10, 2013 at 12:02
  • First ask yourself what is the unit under test. Is it the whole class or a method? That unit needs to be public in order to be unit tested properly. In UI projects, I usually seperate testable logic into controllers/view-models/services that all have public methods. The actual views/widgets are wired to the controllers/view-models/services and tested through regular smoke testing (i.e. start the app and start clicking). You can have scenarios that should test the underlying functionality and having a unit tests on the widgets leads to fragile tests and should be done with care.
    – Spoike
    Commented Jul 10, 2013 at 12:24
  • @Spoike so are you making your VM methods public just to make them accessible to our unit tests? Perhaps that's where I'm going wrong, trying to hard to make everything internal. I think that my hands might be tied with Moq though, which seems to require classes (not just methods) be public in order to mock them (see my comment on Telastyn's answer). Commented Jul 10, 2013 at 12:41

5 Answers 5

8

In general, if you make an interface which only has one implementor, you're just writing things twice and wasting your time. Interfaces alone do not provide loose coupling if they're tightly coupled to one implementation... That said, if you want to mock these things in unit tests, that's usually a great sign that you'll inevitably need more than one implementor in real code.

I would consider it a bit of a code smell if nearly all your classes have interfaces though. That means they're almost all working with one another in one way or another. I would suspect that the classes are doing too much (since there's no helper classes) or you've abstracted too much (oh, I want a provider interface for gravity since that might change!).

7
  • 1
    Thanks for the reply. As mentioned, there are a few misguided reasons why I did what I did, even though I knew that most interfaces would never have more than one implementation. Part of the problem is I'm using the Moq framework which is great for mocking interfaces, but to mock a class it must be public, have a public parameterless ctr, and the methods to be mocked must be 'public virtual'). Commented Jul 10, 2013 at 11:52
  • Most of the classes I'm unit testing are internal, and I don't want to make them public just to make them testable (although you could argue, that's what I've written all these interfaces for). If I get rid of my interfaces then I'll probably need to find a new mocking framework! Commented Jul 10, 2013 at 11:53
  • 1
    @AndrewStephens - not everything in the world needs mocked. If the classes are solid (or tightly coupled) enough to not need interfaces, they're solid enough to just use as-is in unit testing. The time/complexity moq-ing them isn't worth the testing benefits.
    – Telastyn
    Commented Jul 10, 2013 at 13:28
  • -1, since often you don't know how many implementors you need when you first write the code. If you use an interface from the start, you don't have to change clients when writing additional implementors.
    – Wilbert
    Commented Aug 27, 2015 at 7:25
  • 1
    @wilbert - sure, a small interface is more readable than the class, but you're not picking one or the other, you have either the class or a class and an interface. And you might be reading too much into my answer. I'm not saying never make interfaces for a single implementation - most should in fact, since most interactions should require that flexibility. But read the question again. Making an interface for everything is just cargo culting. IoC isn't a reason. Mocks aren't a reason. The requirements are a reason to implement that flexibility.
    – Telastyn
    Commented Aug 27, 2015 at 13:33
4

1) Even if concrete classes are mockable, it still requires that you make members virtual and provide a parameterless constructor, which may be obtrusive and unwanted. You (or new team members) will soon find yourself systematically adding virtual's and parameterless constructors in every new class without a second thought, just because "that's the way stuff works".

An interface is IMO a much better idea when you need to mock a dependency, because it allows you to defer implementation until when you really need it, and makes for a nice clear contract of the dependency.

3) How's that a falsehood ?

I believe interfaces are great for defining messaging protocols between collaborating objects. There might be cases when the need for them is debatable, as with domain entities for instance. However, wherever you have a stable partner (i.e., an injected dependency as opposed to a transient reference) you need to communicate with, you should generally consider using an interface.

3

The idea of IoC is to make the concrete types replaceable. So even if (right now) you only have a single Calculator that implements ICalculator, a second implementation could depend only on the interface and not on implementation details from Calculator.

Therefore, the first version of your IoC-Container registration is the correct one, and the one you should use in future.

If you make your classes public or internal is not really related, and neither is the moq-ing.

3

I'd really be more worried about the fact that you seem to feel the need to "mock" just about every type in your whole project.

Test doubles are mostly useful for isolating your code from the external world (third-party libs, disk IO, network IO, etc.) or from really expensive computations. Being too liberal with your isolation framework (do you even REALLY need it? is your project that complex?) can easily lead to tests that are coupled to the implementation, and it makes your design more rigid. If you write a bunch of tests that verify that some class called method X and Y in that order, and then passed parameters a, b and c to method Z, are you REALLY getting value? Sometimes you get tests like this when testing logging or interaction with third party libs, but it should be the exception, not the rule.

As soon as your isolation framework tells you that you must make stuff public and that you must always have parameterless constructors, it's time to get the heck out of dodge, because at this point your framework is FORCING you to write bad (worse) code.

Speaking more specifically about interfaces, I find they are useful in a few key cases:

  • When you need to dispatch method calls to different types, e.g. when you have multiple implementations (this is the obvious one, since the definition of an interface in most languages just is a collection of virtual methods)

  • When you need to be able to isolate the rest of your code from some class. This is the normal way of abstracting away the file system and network access and the database and so on from the core logic of your application.

  • When you need inversion of control to protect application boundaries. This is one of the most powerful ways possible for managing dependencies. We all know high level modules and low level modules shouldn't know about eachother, because they will change for different reasons and you DON'T want to have dependencies on HttpContext in your domain model or on some Pdf rendering framework in your matrix multiplication library. Making your boundary classes implement interfaces (even if they are never ever replaced) helps loosen the coupling between layers, and it drastically reduces the risk of dependencies seeping through the layers from types knowing of too many other types.

1

I lean towards the idea that, if some logic is private, then the implementation of that logic should be of no concern for anyone, and as such shouldn't be tested. If some logic is complex enough that you want to test it, that logic probably has a distinct role and should therefor be public. E.g. if I extract some code in a private helper method, I find that it's usually something that could just as well sit in a separate public class (a formatter, a parser, a mapper, whatever).
As for interfaces, I let the need for having to stub or mock the class drive me. Stuff like repos, I usually want to be able to stub or mock. A mapper in an integration test, (usually) not so much.

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.