Why you should not place your invariants inside domain services
(Une fois de plus, merci à Jason Tan pour son peer review!)
During the DDD training I give, I always advise people to place their invariants in value objects. If not possible, then use entities, then use aggregate roots, and as a last resort, put it in domain services (and of course, never in application services).
However, when you look at production code, it is not rare to see invariants located in domain services. Why is that? Legacy code can sometime be hard to handle. I can live with that. But what about recently written and well tested code? And what about completely new domain code without any other dependencies?
Even if object-oriented programming (and more specifically the encapsulation principle) has existed for more than 4 decades, that principle is not always well applied. Placing invariants in the middle of a series of steps in a procedural style service is still an approach people use.
Preventing people with bad intentions from doing bad things in a code base is merely impossible. But preventing people with good intentions from making honest mistakes is possible if you organize your code properly.
The major problem with placing invariants in a domain service is that people will not be forced to apply those invariants. Of course, people who know the code base and the domain by heart will not forget that. But all others will fall into the trap. And it was my case last week.
Let me show you a bug I introduced by mistake. First, I will show you a couple of code samples I was dealing with when I introduced the bug. Then I will show you how a simple refactor makes it impossible to make that mistake again.
ServicePeriod class. As you can see, you can provide an Offset and a nullable StartDayOfMonth to the constructor. And an interesting part is at line 20. Looks like there is an IsAlignedOnBillingPeriod concept (whatever that means), which depends on the existence (or nonexistence) of the StartDayOfMonth property.
CustomSubscriptionBundleBuilder class. When you look at that builder, you quickly realize the only thing it does for you is to apply an invariant to know how to assign StartDayOfMonth (line 27) into the ServicePeriod (line 22) I just showed you in the previous class. It now explains the IsAlignedOnBillingPeriod property of the ServicePeriod class (line 20).
And finally, here is the mistake I made. At some point, I needed to add a step to update a bundle’s ServicePeriod. The thing is the bundle update method (same class the CustomerSubscriptionBundlerBuilder is responsible to build the first time you need to create it) accepts the 2 parameters required to instantiate a new ServicePeriod.
So, what mistake did I make? I did not apply the invariant of the CustomerSubscriptionBundlerBuilder class (line 27). Why? Because nothing in the code gave me a hint that a rule like that existed! And even for the creation process, people could easily bypass the builder by calling the CustomSubscriptionBundle constructor directly because the builder/factory does not abstract a creation complexity, it is only the abstraction of a business rule application! At the end, nothing in the code enforces an always-valid domain by forcing invariants to be applied.
Identifying the problem took me 35 minutes.
Fixing the bug and refactoring the code took me 15 minutes.
Code reviewing my work took 5 minutes.
Deploying and validating the code in the testing environment took 15 minutes.
For a total of 70 minutes.
How could better structured code could have prevented my mistake? Here is the way I reworked all this.
First, I made the ServicePeriod constructor private (line 11).
Then I moved the invariant of the builder class inside the ServicePeriod value object (line 34).
And finally, I added a static factory method to help people instantiate a ServicePeriod (line 20).
Another change I made was to modify the UpdateServicePeriod method to accept a ServicePeriod directly. The bundle has absolutely no interest in the internal details of a ServicePeriod neither how to instantiate it. This is basic encapsulation. If one day the internal details of a ServicePeriod changes, there is no reason to have ripple effects in the bundle class. And more importantly, the bundle does not have the responsibility to build a ServicePeriod anymore. No primitive obsession anymore either!
Honestly, my refactor is quite simple. There is nothing to be proud of. However, this small refactor now prevents the whole system to instantiate a ServicePeriod the wrong way. And this is a huge improvement, really. Sometimes, building an always-valid domain is complex because business rules could be hard to apply depending on the current state of the aggregate root boundaries. But in that case, it could have been implemented initially like that with no extra cost, and I would not have lost 70 minutes in my day.
A lot of things can slow down developers and impact the global velocity of a team. As a developer, I am constantly trying to improve the ways we work to accelerate our development. There will always be external factors and reasons out of our hands. But as professionals, we must be vigilant when it comes to the small details in the code base, in the things that we do control. Losing 70 minutes for a thing like that is a great example of the importance of applying good programming practices that existed before we even learned how to tie our shoelaces. I could have saved 14 minutes by just fixing my bug without refactoring the code base to prevent others from falling into the same trap. But this is another responsibility we have as professionals, to leave the code in a better state than you found it. Make this world a little better.