Yes, SOLID is a very good way to design code that can be easily tested. As a short primer:
S - Single Responsibility Principle: An object should do exactly one thing, and should be the only object in the codebase that does that one thing. For instance, take a domain class, say an Invoice. The Invoice class should represent the data structure and business rules of an invoice as used in the system. It should be the only class that represents an invoice in the codebase. This can be further broken down to say that a method should have one purpose and should be the only method in the codebase that meets this need.
By following this principle, you increase the testability of your design by decreasing the number of tests you have to write that test the same functionality on different objects, and you also typically end up with smaller pieces of functionality that are easier to test in isolation.
O - Open/Closed Principle: A class should be open to extension, but closed to change. Once an object exists and works correctly, ideally there should be no need to go back into that object to make changes that add new functionality. Instead, the object should be extended, either by deriving it or by plugging new or different dependency implementations into it, to provide that new functionality. This avoids regression; you can introduce the new functionality when and where it is needed, without changing the behavior of the object as it is already used elsewhere.
By adhering to this principle, you generally increase the code's ability to tolerate "mocks", and you also avoid having to rewrite tests to anticipate new behavior; all existing tests for an object should still work on the un-extended implementation, while new tests for new functionality using the extended implementation should also work.
L - Liskov Substitution Principle: A class A, dependent upon class B, should be able to use any X:B without knowing the difference. This basically means that anything you use as a dependency should have similar behavior as seen by the dependent class. As a short example, say you have an IWriter interface that exposes Write(string), which is implemented by ConsoleWriter. Now you have to write to a file instead, so you create FileWriter. In doing so, you must make sure that FileWriter can be used the same way ConsoleWriter did (meaning that the only way the dependent can interact with it is by calling Write(string)), and so additional information that FileWriter may need to do that job (like the path and file to write to) must be provided from somewhere else than the dependent.
This is huge for writing testable code, because a design that conforms to the LSP can have a "mocked" object substituted for the real thing at any point without changing expected behavior, allowing for small pieces of code to be tested in isolation with the confidence that the system will then work with the real objects plugged in.
I - Interface Segregation Principle: An interface should have as few methods as is feasible to provide the functionality of the role defined by the interface. Simply put, more smaller interfaces are better than fewer larger interfaces. This is because a large interface has more reasons to change, and causes more changes elsewhere in the codebase that may not be necessary.
Adherence to ISP improves testability by reducing the complexity of systems under test and of dependencies of those SUTs. If the object you are testing depends on an interface IDoThreeThings which exposes DoOne(), DoTwo() and DoThree(), you must mock an object that implements all three methods even if the object only uses the DoTwo method. But, if the object depends only on IDoTwo (which exposes only DoTwo), you can more easily mock an object that has that one method.
D - Dependency Inversion Principle: Concretions and abstractions should never depend on other concretions, but on abstractions. This principle directly enforces the tenet of loose coupling. An object should never have to know what an object IS; it should instead care what an object DOES. So, the use of interfaces and/or abstract base classes is always to be preferred over the use of concrete implementations when defining properties and parameters of an object or method. That allows you to swap one implementation for another without having to change the usage (if you also follow LSP, which goes hand in hand with DIP).
Again, this is huge for testability, as it allows you, once again, to inject a mock implementation of a dependency instead of a "production" implementation into your object being tested, while still testing the object in the exact form it will have while in production. This is key to unit testing "in isolation".
Having one instead of many parameters in the constructor is not the problematic part of this design. As long as your IContext
class is nothing but a service facade, specificially for providing the dependencies used in MyControllerBase
, and not a general service locator used throughout your whole code, that part of your code is IMHO ok.
Your first example might be changed to
protected MyControllerBase(IContext ctx)
{
TheContext = ctx;
SomeService = ctx.GetSomeService();
AnotherService = ctx.GetAnotherService();
}
that would not be a substantial design change of MyControllerBase
. If this design is good or bad depends only on the fact if your want to
- make sure
TheContext
, SomeService
and AnotherService
are always all initialized with mock objects, or all of them with real objects
- or, to allow initialization them with different combinations of the 3 objects (which means, for this case your would need to pass the parameters individually)
So using only one parameter instead of 3 in the constructor can be fully reasonable.
The thing which is problematic is IContext
exposing the GetService
method in public. IMHO you should avoid this, instead keep the "factory methods" explicit. So will it be ok to implement the GetSomeService
and GetAnotherService
methods from my example using a service locator? IMHO that depends. As long the IContext
class keeps just beeing a simple abstract factory for the specific purpose of providing an explicit list of service objects, that is IMHO acceptable. Abstract factories are typically just "glue" code, which don't have to be unit-tested itself. Nethertheless you should ask yourself if in the context of methods like GetSomeService
, if you really need a service locator, or if an explicit constructor call would not be simpler.
So beware, when you stick to a design where the IContext
implementation is just a wrapper around a public, generic GetService
method, allowing to resolve any arbitrary depencencies by arbitrary classes, then everything applies what @BenjaminHodgson wrote in his answer.
Best Answer
I went and read the blog post, and I agree with a lot of what the author said. However, if you're writing your code using interfaces for unit testing purposes, I'd say that the mock implementation of the interface is your second implementation. I'd argue that it really doesn't add much in the way of complexity to your code, especially if the trade-off of not doing so results in your classes being tightly coupled and hard to test.