Hey, catchy title, huh? Before the flame wars start, I'm not saying that all business rules are poorly written, nor am I saying that I came up with a magic solution. Before I even begin talking about my idea let me give you some background.
Background
In my previous project, the domain was fairly complex for 3 different communicating applications. As it happens in any line-of-business application, the business rules had several conditions to be met before they could be executed. What I mean with conditions is pre-requisites for the rule being run.
There are some possible problems with that approach (all of which I could identify in the 3 different codebases we had):
- The code is harder to read, since it's very nested (think if's, else's, switches or any other branching mechanism) and intertwined with calls to methods in different classes. This, eventually makes the actual business rules behind a given service call impossible to decipher for most of the original team members, not to mention for new people joining.
- Unit testing these rules is REALLY hard, since you have to come up with all sorts of different scenarios that can match all the possible combinations of pre-requisites and results. This, in turn, results in less tests and "coverage mislead"*.
- Makes the service classes (or whatever classses you put your business rules in) really big, really fast, since all the branches are in the same file. This has diverse negative effects like:
- Hard to parse files if you use tools like resharper. You could split this into more files, but that just makes item 2 worse.
- Hard to understand files. Small files are easier for us to read then big ones. If I want a book, I'll go read one.
- Yuck effect. Related to 3.2. This is the general feeling of someone joining the project. I consider morale to be one huge aspect of every agile team. Having huge files makes developers want to cry, thus hurting the team morale pretty bad. People already in the team feel bad about it and people joining feel bad about complaining (not me though, lol).
As you can see, these are some pretty serious issues. I haven't worked in a single project where this does not repeat itself, which is weird. I consider this to be my fault for not noticing these issues earlier, or failing to take action to prevent that.
What annoys me the most is that I only acknowledged all of that after reading this very good post by Ayende (http://ayende.com/Blog/archive/2008/11/23/aspects-of-domain-design.aspx). Anyway, better late then never.
How to prevent it
Ok, now that we know some of the issues, I want to think about what are business rules and what are they composed of. No, I'm not starting a business rule farm and wondering what they eat! I just need to think of how we can split them in a way that's intuitive, readable and testable.
From what I gathered, business rules are made of:
- Requirements
- Data Manipulation
"Well, duh! Why am I even reading this?! Everyone knows that!"
Yeah I know, dear reader, but sometimes you have to look at the obvious concepts to acknowledge that you need to dig a little deeper. With that in mind, let's define both concepts.
Requirements are pre-conditions that need to happen in order for a given business rule to be executed. These are really common and tend to change fairly often as the project evolves.
Data Manipulation are actions that change application state like ORM calls, database calls and whatever else changes any state in the application or the domain.
"Still stating the obvious... Boooring..."
Ok, calm down! I'm getting there! Given that we agree with these two definitions, what do you say we split them?
"Uh?"
Yes, let's get rid of all the requirements in our business rules and dump them somewhere else. That way our business rules can stick to what they should be doing, changing state.
"Wait, I heard that before. Please say it's not another Business Rules Engine! I don't even want to read any further!"
Please keep reading! I swear it's not just another business rules engine. Bear with me just for a while!
Getting rid of Requirements - Learning by Example
I do believe that the best way of learning something is by example. Sometimes it's very hard to grasp some concepts just by themselves. Having an example really helps. So let's get one going.
Let's assume that we have this service:
The business rules requirements are:
- For Debit (takes an account and an amount)
- The account needs to be active
- The amount being debited must be lower than the amount in the account (you can't take more money than you have)
- For Transfer (takes from account, to account and amount)
- Both accounts need to be active
- The amount being transferred must be lower than the amount in the from account (due to the rule that you can't take more money than you have)
- The amount to be transferred must be greater than 10 (we are ignoring currency for this example).
Pretty simple rules, huh? Let's check at how an implementation of this would be:
1: public class ClassicAccountManagementService : IAccountManagementService
2: { 3: public void Debit(Account account, decimal amount)
4: { 5: if (account.AccountStatus!=AccountStatus.Active)
6: throw new InvalidOperationException("The account must be active in order for an operation to be performed."); 7: if (amount > account.Amount)
8: throw new InvalidOperationException("The account does not have enough money for a debit operation."); 9:
10: account.Amount -= amount;
11: }
12:
13: public void Transfer(Account from, Account to, decimal amount)
14: { 15: if (from.AccountStatus != AccountStatus.Active || to.AccountStatus != AccountStatus.Active)
16: throw new InvalidOperationException("The account must be active in order for an operation to be performed."); 17: if (amount > from.Amount)
18: throw new InvalidOperationException("The account does not have enough money for a debit operation."); 19: if (amount < 10)
20: throw new InvalidOperationException("To transfer money between accounts the minimum amount is 10."); 21:
22: from.Amount -= amount;
23: to.Amount += amount;
24: }
25: }
You can easily see that lines 5-8 and 15-20 are requirements for the rules and lines 10 and 22-23 are the data manipulation actions for the rules.
How about we change that to read like this:
1: public class AccountManagementService : IAccountManagementService
2: { 3: public void Debit(Account account, decimal amount)
4: { 5: account.Amount -= amount;
6: }
7:
8: public void Transfer(Account from, Account to, decimal amount)
9: { 10: from.Amount -= amount;
11: to.Amount += amount;
12: }
13: }
Hey, that looks good! Except it does not meet our criteria for the business rules. We would have a lot of failing tests with this. I still like it a lot, though.
Specifying Requirements
Usually when I'm trying out some new idea I tend to write code that matches how I would like to use it, then make it work. Sometimes I find some spikes down the road, but usually this approach works for me.
Let's write the code for our service the way I think I would like to read:
1: public class AccountManagementService : IAccountManagementService
2: { 3: [IsSatisfiedBy(AccountIsActive.Requirement)]
4: [IsSatisfiedBy(AccountHasEnoughMoneyForDebit.Requirement)]
5: public void Debit(Account account, decimal amount)
6: { 7: if (Context.Failed) return; //means some of the requirements were not met. Could throw instead.
8:
9: account.Amount -= amount;
10: }
11:
12: [IsSatisfiedBy(FromAccountIsActive.Requirement)]
13: [IsSatisfiedBy(ToAccountIsActive.Requirement)]
14: [IsSatisfiedBy(FromAccountHasEnoughMoneyForDebit.Requirement)]
15: [IsSatisfiedBy(TransferMustMeetMinimumValue.Requirement)]
16: public void Transfer(Account from, Account to, decimal amount)
17: { 18: if (Context.Failed) return; //means some of the requirements were not met. Could throw instead.
19:
20: from.Amount -= amount;
21: to.Amount += amount;
22: }
23: }
Now, that's a lot closer to what I'd like to read. Some clear advantages of this code:
- It's easier to read. You can clearly see what each rule does and at the same time know all the pre-requisites that need to be in place for this rule to be executed properly. What this means is that when you get back to this code 6 months from now (and if you don't someone will), you can still easily understand what it does and why.
- This code is a lot more testable. Writing simple code pays off when you have to test it. Since I abstracted all the requirements to the business rule, I got left only the Data Manipulation actions. What that means is that my unit tests are as simple as checking that when executed, this method changes the data in the expected way.
- Another point for testability. Since each requirement has been abstracted to its own class, I can test them in isolation, making writing scenarios for them a lot simpler.
Let's see the actions now:
1: public class AccountIsActive : IRequirement
2: { 3: public const string Requirement = "AccountIsActive";
4:
5: public void Execute(Account account)
6: { 7: if (account.AccountStatus != AccountStatus.Active)
8: Context.FailWith("The account must be active in order for an operation to be performed."); 9: }
10: }
1: public class AccountHasEnoughMoneyForDebit : IRequirement
2: { 3: public const string Requirement = "AccountHasEnoughMoneyForDebit";
4:
5: public void Execute(Account account, decimal amount)
6: { 7: if (amount > account.Amount)
8: Context.FailWith("The account does not have enough money for a debit operation."); 9: }
10: }
1: public class TransferMustMeetMinimumValue : IRequirement
2: { 3: public const string Requirement = "TransferMustMeetMinimumValue";
4:
5: public void Execute(decimal amount)
6: { 7: if (amount < 10)
8: Context.FailWith("To transfer money between accounts the minimum amount is 10."); 9: }
10: }
As you can see the actions are pretty simple and very, very easy to test. Now that we got all of them in place, finally, I'll explain my idea.
Wiring it all together
Whenever I start a new project the first infrastructure component I setup is the IoC container. There's a lot of friction involved in managing components dependencies, and I want all of that to stay the hell away from my production code.
Since my tool of choice is Windsor, it won't surprise you that the only container I'm integrating with so far is Windsor. The structure I built, as you'll see, is decoupled enough that it should be straightforward writing the other container's integration.
Let me show a test that checks that you can't transfer money from one account to another because the from account is not active.
1: [Test]
2: public void ShouldNotTransferBetweenAccountsSinceFromAccountIsInactive()
3: { 4: AchloraContext.ThrowOnError = false;
5: var expectedError = new System.Collections.Generic.List<string> { "The account must be active in order for an operation to be performed." }; 6: var from = new Account(500, AccountStatus.Inactive);
7: var to = new Account(500, AccountStatus.Active);
8:
9: var service = IoC.Resolve<IAccountManagementService>();
10:
11: service.Transfer(from, to, 200);
12:
13: Assert.That(AchloraContext.Failed, "It should've failed since the from account is not active.");
14: Assert.That(from.Amount, Is.EqualTo(500.00M));
15: Assert.That(to.Amount, Is.EqualTo(500.00M));
16: Assert.That(AchloraContext.Errors, Is.EquivalentTo(expectedError));
17: }
Let's tackle each line at a time.
At line 4 we have AchloraContext. So far I haven't talked about a project have I? Yeah I have. In the title. The project name is Stormwind.Accord (I told you it was catchy!), codenamed Achlora (just for the proof of concept). This context class is responsible for letting our methods know about what happened with the requirements. This is really useful if you don't want to throw automatically, or you need to inspect the results of the requirements.
In lines 5 to 11, we setup some test data and call our service method passing the data.
From lines 13 to 16 we assert that the requirements were executed properly and that no data was changed due to requirements not being met.
Well, moving on...
Windsor Integration
"WAIT JUST ONE SECOND THERE!!!"
What? What's going on?
"How the hell did those requirements get executed in the Transfer method? I didn't see any calls to Requirement.Execute!"
Oh, sorry, my bad! I forgot to explain that since I really like Windsor, we are fully integrated to it and because of that we intercept your calls (if you added any IsSatisfiedBy attributes). The requirements then get executed before executing your method. That's how you get your nice context in the method.
The only requirements for all this magic is that you register a facility in the container and all the requirements in the container.
"Why would I want to register requirements in the container?"
Hmmm... So the container can manage their dependencies as well? Say you have a requirement that needs to query some repository on some database stuff (pretty common, huh?). If your requirement is resolved from the container, all you have to do is inject the repository into your requirement, and off you go.
Just as a sample, this is the code that sets up the container for the tests that I've written for Accord:
1: [TestFixtureSetUp]
2: public virtual void TestFixtureSetUp()
3: { 4: container = new WindsorContainer();
5: container.AddFacility(WindsorIntegrationFacility.Key, new WindsorIntegrationFacility());
6: container.Kernel.Register(AllTypes
7: .FromAssembly(typeof(AchloraTest).Assembly)
8: .BasedOn<IService>()
9: .WithService.FirstInterface());
10:
11: container.Kernel.Register(AllTypes
12: .FromAssembly(typeof(AchloraTest).Assembly)
13: .BasedOn<IRequirement>()
14: .Configure(registration => registration.Named(GetKeyFor(registration.ServiceType))));
15: }
As you can see, not that hard to setup, now is it?
Conclusion
There's a lot more than just that, since we want to reuse rules (why have an IsAccountActive, IsFromAccountActive, IsToAccountActive, when we can have just AccountIsActive and just call the right arguments). That is implemented with parameter mapping as you can see in the test cases for the tool.
If you'd like to take a look, it's in my personal area right now under the codename Achlora. The project is completely self-contained, meaning you can just download it and try it. As usual the code is provided "as-is" with no warranties whatsoever, bla bla bla. Read more about it in Stormwind License. This one is still not even alpha. It's just a proof of concept. Gotta do some polishing in it to turn it to a Stormwind project.
I'd really like some feedback on this, since I feel it's an issue that scares most devs and plagues most projects.
I might not even be on the right track, but please just give me some honest feedback, it really helps! I'd be most thankful if you take 10 minutes to just download the code and try it. Just read, think about it and if you do like it talk to me about contributing! (oh damn, it's not even a project yet and I'm already asking for contributors! It's force of habit)
The code can be checked out from http://svn.stormwindproject.org/svn/Personal/Heynemann/Achlora.
Happy coding!
PS: Sorry for all the bad jokes, lol... I just can't resist!
* Even assuming you got good coverage, you'd hardly tested the rule properly for all different combinations of inputs, which lessens the confidence that it's going to work in a production environment. We all know users come up with the most unlikely combination of input data. That reminds me I should post about coverage mislead.