1

I have been looking around for some practices to write testable code and gathered the following:

• Public methods virtual if not using interfaces -- makes mocking easier
• Dependency injection -- makes mocking easier
• Smaller, more targeted, choesive methods -- tests are more focused, easier to write
• Avoidance of static classes
• Avoid singletons, except where necessary
• Avoid sealed classes

I am not sure if I get the point of the first one, making public methods virtual, should I really do that?

  • If you make a public method also virtual in order to mock it in a unit tests then this is a code smell. A possible refactor might be to extract the method's logic into a specialized entity (which can be another standalone class, interface with a concrete implementation or a completely entire module). – TeoMor Feb 2 '18 at 8:02
  • @Laiv Not really, I am just curious to learn about all these things around, such as testability. – Ezoela Vacca Feb 2 '18 at 8:09
  • Ok. But the rationale still is the same. If you miss the goal you will end up with Golden Hammers. I do tell you this because the way you stated the question should I ...? – Laiv Feb 2 '18 at 8:16
  • @Laiv I understand and get your point. I found very useful SOLID principles, not because I want to follow them zealously but because I can study problems they solve. Similarly I would like to know general practices for writing good unit tests, if something that generic exists. – Ezoela Vacca Feb 2 '18 at 8:18
  • Not it doesn't exist. Every case is going to be subordinated to certain requirements, needs, constraints, etc. That's why usually, question starting with should I are dangerous to answer. They become a pure guesswork. – Laiv Feb 2 '18 at 8:21
4

Public methods virtual if not using interfaces -- makes mocking easier

Do not focus on mocks. Mocks are a valuable testing tool, but every time you create a mock, you are testing your code against an artificial environment, rather than the one it'll face in production. So minimise your use of mocks. To that end, marking methods as virtual really is a test tool of last resort. Overriding a method in order to test can often cause more problems than it solves (the tests all too easily break when changes are made to the way the two methods interact in the real code for example). Instead, use interfaces in the first place for when you do need to mock.

Dependency injection -- makes mocking easier

No! Stop fixating on mocking. Dependency injection helps reduce coupling between parts of your code, making both maintenance and testing far easier. Mocking is only made easier (if needed) by referencing those injected dependencies via interfaces.

Smaller, more targeted, choesive methods -- tests are more focused, easier to write

Smaller, more cohesive methods have many advantages. They tend to be far simpler to understand for a start. They can indeed also be easier to test. This one is definitely good advice.

Avoidance of static classes

Static classes only need to be avoided when their methods have side effects (access or modify global state, external resources like the file system, databases etc). If the methods are pure, ie rely on only their inputs and constants to derive a deterministic result, then make them static.

[Addendum, as a nod to something I learned from RobertHarvey recently, beware using static for methods that take a long time to execute. Unit tests should be fast and may need to mock slow methods, so don't make those methods static]

Avoid singletons, except where necessary

Avoid (in fact, never use) the singleton pattern. It's a glorified global variable. It's an anti-pattern. It's never ever necessary to use it.

Single instances of classes, injected into those parts of the code that need access absolutely should be used though. Please don't end up creating two copies of a class just to avoid being accused of using a singleton for example.

Avoid sealed classes

If a class is badly designed, doesn't use an interface and has virtual methods so they can be overridden by tests, then this holds true. But you don't want that; you want well designed classes that are accessible via interfaces and that do not contain virtual methods. At which point you can seal all of them. I do.

  • 2
    But why not use mocking? Several books dealing with unit testing describe it. If I need to test a class in isolation, I do need to fake dependencies somehow – Ezoela Vacca Feb 2 '18 at 8:26
  • 1
    @EzoelaVacca, I'm not saying don't use it. Instead I'm saying be aware of the cons as well as the pros of using it and only use it when necessary, eg to isolate a test from directly accessing a database or to replace a long-running method with a fast mock to avoid slow unit tests. – David Arno Feb 2 '18 at 8:29
  • 1
    I think you didn't get David's point. You can mock or stub components and behaviours, but don't make these things the core of the design. – Laiv Feb 2 '18 at 8:29
1

I'm not sure I agree with the previous answer so I'm going to attempt my own. I think the point the the previous poster is trying to make is that you should focus on code quality and testability rather than following these as hard and fast rules. What I'd add to that is that just in manual QA, when writing automated tests (such as unit tests, integration tests, etc) the overall code quality and test strategy is often more important than the bullet points.

What you've found is a series of guidelines which you can implement in your code to make it more testable. I think this is actually coming at the problem backwards - think about your tests, THEN think about your code. This is known as Test Driven Development or TDD.

Let's talk about why we write automated tests for our code at all. We write them because we don't want our methods to change behaviour unexpectedly. Go back to your SOLID Principles and think about "O" - we want our code to be "Open to Extension but Closed to Changed"

The Open/Closed principal encourages us to write code which can be extended without needing to change existing code. This is because changing the existing of code which is already in production brings risk with it, one strategy to mitigate this risk and prevent the introduction of bugs is to test our code (either manually or in an automated manner) and one of the most popular methods of doing this is with Unit Tests.

Unit Tests break your code into it's smallest units and test the behaviour of each method/operation in isolation. If the functionality of each method remains unchanged then the larger application will not change unexpectedly and fewer bugs should be introduced.

However, Unit Tests are not going at detecting bugs in code - they only help to enforce this Open/Closed principal and prevent bugs being introduced into already working code. That's why Unit Tests should form part of your overall test strategy, they're not a silver bullet.

Having established the value of Unit Tests let's talk about TDD.

My main issue with the bullet points above is that they imply that you're writing code which can then be tested at a later date, a far better approach is to:

  • Create a method stub which contains the signature and throws something like a NotImplementedException
  • Create a test class which represents this method
  • Create tests which "document" what the method does in each scenario, this will include scenarios like the happy path, passing in nulls, passing in out of range values, and potentially internal errors
  • Implement your method and ensure all the tests pass

Creating a test to pass in a null is fairly easy:

[Test]
public void An_exception_is_thrown_when_null_is_supplied()
{
  var myClass = new MyClass();
  Assert.Throws<ArgumentNullException>(() => myClass.DoSomething(null));
}

However testing what will happen if there's an error inside your class (for example accessing the database) is where things get a little more complicated and that's where mocks and stubs come in. Following a dependency injection (or DI pattern) we can write a test which says:

[Test]
public void An_exception_is_thrown_when_the_database_cannot_be_contacted()
{
  var myDataAccess = new Mock<IDataAccess>();
  myDataAccess.Setup(x => ConnectToDatabase()).Thows<InvalidOperationException>());
  var myClass = new MyClass(myDataAccess.Object);

  Assert.Throws<InvalidOperationException>(() => myClass.DoSomething("user1"));
}

Now I've waffled and built up some background let's look at your first bullet point (the one you're asking about).

Public methods virtual if not using interfaces -- makes mocking easier

In our implementation we can assume MyClass looks something like this:

public class MyClass
{
  public MyClass(IDataAccess dataAccess)
  {
     // Null check dataAccess and assign to an internal variable somewhere

However, what if (for whatever reason) you can't pass in the dependency?

What if your MyClass implementation looks like this:

public class MyClass
{
  public void DoSomething(string username)
  {
    throw new NotImplementedException();
  }

  public void ConnectToDatabase()
  {
    // connect to the database

Our mocking libraries are going to find it VERY hard to simulate an error when connecting to the database. However some mocking libraries allow you to mock virtual methods by returning a class which inherits from your class and overrides the virtual methods. This would mean you could mock out the ConnectToDatabase() method forcing it to throw an exception for you. However the mocking library will not be able to do this if if the class is sealed (the last bullet point) or if the methods are not virtual.

Hopefully that helps explain why your bullet list isn't ideal, the rational for the first bullet point and how I'd do things instead?

TLDR

Making methods virtual helps mocking libraries override them if DI cannot be used.

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.