State of Application Design

In vast majority of applications I have seen the domain logic is implemented using a set of Service classes (Transaction Scripts). Majority of these are based on the DB structure. Entities are typically quite thin DTOs that have little or no logic.

The main benefit of this kind of architecture is that it is very simple and indeed often good enough as a starting point. However, the problem is that over time when the application gets more complex this kind of approach does not scale too well. Often you end up with Services that call 6 - 8 other Services. Many of these Services have no clear responsibilities but are built in an ad-hoc manner as wrappers of existing Services adding tiny bits of logic needed for some specific new feature.

So how to avoid or dig yourself out from this kind of architecture? One approach I have found very useful is looking at the unit tests when writing them. By listening to what my tests are trying to tell me I will be able to build much better design. This is nothing else but the "Driven" part in TDD which everybody knows but is still quite hard to understand.

Indeed it is quite easy to write tests before production code but at the same time not let these tests have any significant effect on production code. Sometimes there is also this thinking that testing is supposed to be hard in which case it is particularly easy to ignore the "smells" coming from tests.

Following are some rules I try to follow when writing tests. I have found that these ideas help me to avoid fighting my tests and as a result not only are tests better but also the production code.

In the following text I use "spec" to refer to a single test class/file.

Rule 1: when spec is more than 120 lines then split it

When the spec is too long I will not be able to grasp it quickly anymore. Specific number does not matter but I have found around 120 lines to be a good threshold for myself. With very large test file it gets hard to detect duplication/overlap when adding new test methods. Also it becomes harder to understand the behavior being tested.

Rule 2: when test names have duplication it is often a sign that you should split the spec

Typically unit tests are 1:1 mapped to each production class. So tests often need to specify what exact part of the target class is being tested. This is especially common for the above mentioned Services which are often just collections of different kinds of procedures.

Lets say that we have a PaymentMethodService which has tests like:

def "when gets payment methods for EUR then returns single card method"()
def "when gets payment methods for non-EUR then returns debit and credit as separate methods"()
def "when gets payment methods then returns only enabled methods"()
def "when gets payment methods for a known user then orders them based on past usage"()
def "when gets payment methods for transfer amount > 2000 GBP then returns bank transfer as the first method"()

These tests all repeat when gets payment methods. So maybe we can create a new spec for getting payment methods and we can just dump the duplicating prefix from all of the test names. Result will be:

class GetPaymentMethodsSpec {
  def "returns only enabled methods"()
  def "when user is known then orders methods based on past usage"()
  def "for transfer amount > 2000 GBP bank transfer is the first method"()

Note that the spec name does not contain name of any production class. If I can find a good name that contains tested class I don't mind but if it gets in the way then I'm willing to let go of the Ctrl+Shift+T. This aligns with the idea of Uncle Bob that test and production code evolve in different directions.

Rule 3: when you have split too long spec then always think whether you should split/extract something in production code as well

If there are many tests for something then it means that the tested behavior is complex. If something is complex then it should be split apart. Often lines of code are not good indicator for complexity as you can easily hide multiple branches/conditions into single line.

From the previous example if we have multiple tests around the ordering of payment methods it may be a good sign that ordering could be extracted into a separate class like PaymentMethodOrder.

Rule 4: when test contains a lot of interactions then introduce some new concept in the production code

When looking at the tests for such Transaction Script Services then often they contain a lot of interactions. This makes writing tests very hard as it should be because there is clearly too much going on at once and we are better off splitting it.

Rule 5: extract new class when you find yourself wanting to stub out a method in tested class

When you think that you need to mock/stub some class partially then this is generally bad idea. What the test is telling you is that you have too much behavior cramped together.

You have 2 choices:

  • don't mock it and use the production implementation
  • if your test becomes too complex or you need too many similar tests then extract that logic out into separate class and test that part of behavior separately

You can also check out my post from few years ago for more tips for writing good unit tests.

Used still from Ridley Scott's Blade Runner