Using the Rules Pattern to Improve Code Maintainability
Picture this scenario; you have been tasked with writing some code that creates a sales order in the system. You choose to use a standard BAPI provided by SAP. Nice, clean code that is running in production.
Then, a few weeks later, an application consultant informs you that the business needs have changed for one customer only that uses the solution; they need a check first that the material ordered is in stock in a quantity suitable for one delivery – don’t create the order if that is not possible.
So you modify your original solution and add a bit of complexity. Not much, just one ‘IF’ branch and some handling logic for that one customer only. No problem.
The next week a new requirement appears in your email; If the delivering plant is greater that 500 km from the Ship-to party and the order is of a certain type, do not create the order. More checking code goes into your original, elegant solution.
Two weeks later, another check requirement to implement; a month later one more. By now, your code is beginning to ‘smell’ a bit – not your fault, how could you have known these little checks and subsequent implementations were going to add additional complexity to your once clean code, whose only single responsibility originally was to create sales orders from the data provided.
One year down the line of this, and the code has gone from being somewhat inelegant to virtually unmaintainable; the checks have formed a morass of spaghetti code that is hard to read through and understand what all of the business requirements were. Any additional logic may introduce unintended side effects that slip through regression testing, because there are so many paths through the code that testing becomes a nightmare; consultants have moved on, developers have moved on, and understanding the complexity of the code is a tall order indeed.
Not only that, but every time the code is modified, it means that the source code is opened up, edited and transported, adding additional risks that once tested code is inadvertently changed in an unintended manner, with the subsequent post-mortem to discover what actually went wrong after the code has been released to the productive environment.
The Root Cause
What is the root cause of all of this mayhem? In a word, unpredictability. It was impossible to gather all of the requirements at design time, and if working according to agile methodologies this would not even be attempted. Requirements evolve over time, new customers and business conditions come onboard, so even if the requirements analysis had been completely comprehensive at the beginning of the design process, new requirements would invariably crop up.
In any case why leave it to the original code to do all of the checks? After all, it was designed to do one thing: create sales orders. Making it do anything else violates the first principle of SOLID design; the Single responsibility principle, which states that:
“There should never be more than one reason for a class to change.”
What if there was a way to separate all of these code checks out into smaller, independent units of code that could be swapped in and out of use, without recourse to opening up the original code and recompiling it? What if those checks could be re-used, eliminating repeated code? What if some customers needed some checks that others did not.
What if the code could be prevented from the ravages of code rot from the outset?
Enter the Rules Pattern
I am indebted to Steve Smith who is the originator of this design pattern.
The rules pattern was specifically designed to address this scenario:
- Code is becoming excessively complex; additional check logic consistently gets added to the original functionality
- Code with one original responsibility gets tasked wish additional concerns relating to executing additional tasks
- The logic for executing these additional tasks gets intertwined with the original code
My implementation of the rules pattern goes a little further than the original pattern, so I’ll break it down into sections, then see how they all work together at the end. At the core of all of this is a single component conceptually identified as a Rule.
A rule is a fundamental, atomic check for a specific condition or state of a system. In plain English, it’s a check that a certain condition can be fulfilled.
Rules typically need to be set up, i.e. be provided with information about the state of the system, and executed, to evaluate the outcome of the rule.
The basic architecture is shown below.
Let’s have a look at the salient features of the diagram above. We have:
The contract definition for the rules. Contains two methods: PREPARE() and EXECUTE().
PREPARE(): Prepare the rule. Pass in any needed data that is necessary for the EXECUTE method.
Here’s an example of a simple PREPARE() statement:
METHOD zif_cssm_rule~prepare. DATA: r_cssm_dlvrep TYPE REF TO zcl_cssm_dlvrep. * We know this is to recieve a message type ZCL_CSSM_DLVREP, * And we need to access some of it's attributes, so narrow the cast. r_cssm_dlvrep ?= im_message. me->set_sales_order( EXPORTING im_sales_order = r_cssm_dlvrep->order ). me->set_sales_order_item( EXPORTING im_sales_order_item = r_cssm_dlvrep->order_item ). ENDMETHOD.
The cast is due to the fact that I am passing in an object of a known type that needs to be explicitly stated in this example. The important point is that I am just setting a sales order document number and sales order item number in this statement, nothing more.
EXECUTE(): Execute the rule. Note the parameters that the method returns:
EX_STATE returns one of three levels: PASS, FAIL or WARN, referring to the state of the rule after evaluation.
EX_ABORT is an indication that whatever happens with the other rules, setting this to ‘TRUE’ is enough to prevent any further rule checks since whatever happens; it’s a kind of system level ‘all bets are off!’ indicator.
There is a reason for this; in my implementation, if a rule fails, I may wish to carry on processing but perhaps skip an operation (say, creating a sales order, because one already exists); on the other hand, if the method returns EX_ABORT = ‘TRUE‘, it means just give up on processing because whatever happens, the process will not succeed (say, a vendor batch number is passed in that does not match a batch in the system). I could have added another level to EX_STATE, but I find separating the signals out this way clearer to understand in the program.
Here is an example of a simple EXECUTE() statement:
METHOD zif_cssm_rule~execute. * Check that referenced sales order exists DATA: lv_vbak TYPE vbeln_va. ex_state = zif_cssm_rule~c_pass. SELECT SINGLE vbeln FROM vbak INTO lv_vbak WHERE vbeln = sales_order. IF sy-subrc NE 0. ex_state = zif_cssm_rule~c_fail. ex_abort = zif_cssm_rule~c_true. ENDIF. ENDMETHOD.
A couple of points to note:
- Obeying the SOLID Single Responsibility Principle, it should be clear that this rule is doing one thing and one thing only – checking that a sales order exists in the database.
- This code looks almost ridiculously trivial, and that’s exactly how it should be; easy to understand. Imagine 20 rules like this all incorporated into the original source code to create a sales order, along with the conditions to determine if a particular rule should be executed for a particular customer; then the reason for organising the code in this manner perhaps starts to make sense.
Abstract Class ZCL_RULE
Although not strictly needed, since any class can implement the needed interface ZIF_RULE,
the abstract class ZCL_RULE can be used to implement the interface and all other rules can inherit from it; this leads to a neat way to implement new rules without the need to recompile existing code, as I will show in a later blog.
Concrete Classes ZCL_RULE_1…ZCL_RULE_n
These are the classes that implement a particular rule.
The Rule Builder and Rule Builder Factory
The Rule Builder
The Rule Builder does just that – builds the rules to be used.
The purpose of this interface is to put the rules together for use in a specific code location.It contains two methods: BUILD_RULES() and GET_RULES().
BUILD_RULES() creates an instance of each rule that is to be executed, and invokes the method ZIF_RULE~PREPARE() on each rule.
Here’s an implementation of BUILD_RULES():
METHOD zif_orders_rule_builder~build_rules. DATA: r_orders_rule TYPE REF TO zcl_orders_rule. * Check that sales order DOES NOT exist for specified purchase order. CREATE OBJECT r_orders_rule TYPE zcl_orders_rule_1. r_orders_rule->prepare( EXPORTING im_message = im_orders ). APPEND r_orders_rule TO order_check_rules. * Check that there are not multiple sales order for specified purchase order - fatal error. CREATE OBJECT r_orders_rule TYPE zcl_orders_rule_2. r_orders_rule->prepare( EXPORTING im_message = im_orders ). APPEND r_orders_rule TO order_check_rules. ENDMETHOD.
Note here that:
- Adding a rule is just a case of creating an instance of a specific rule type, calling PREPARE() on it and adding it to a table of reference to rules.
- Each rule knows how to prepare itself, so other code does not need to do that for the rule.
GET_RULES() returns the list of rules, ready to execute. This is very straightforward:
METHOD zif_cssm_orders_rule_builder~get_order_rules. ex_order_check_rules = order_check_rules. ENDMETHOD.
The Rule Builder Factory
Responsible for deciding which rule builder to build, based on arbitrarily chosen input parameters; it could be anything needed to discriminate which Rule Builder to fabricate. It only has one method – MAKE_ZCL_RULE_BUILDER(), returning the appropriate Builder.
Here’s an example of the implementing code that uses an EDI partner as a parameter:
METHOD make_zcl_rule_builder. CASE im_partner. WHEN 'KRY'. CREATE OBJECT ret_zcl_cssm_rule_builder TYPE zcl_rule_blder_kry. WHEN 'PODEMO'. CREATE OBJECT ret_zcl_cssm_rule_builder TYPE zcl_rule_blder_podemo. WHEN OTHERS. CREATE OBJECT ret_zcl_cssm_rule_builder TYPE zcl_rule_blder_gen. ENDCASE. ENDMETHOD.
Note the following:
- Any specific partner can have their own specific rule builder implementation, meaning that if a particular customer has specific rules to check, they can be instantiated for that customer only.
- Customers having their own rules builder dramatically reduces code complexity; imagine having one universal rule builder that had to check what rule to implement based on customer and possibly many other parameters.
- Note the WHEN OTHERS statement. This is important; it means that for any customer that does not have any specific rule checks, a generic rule builder is instantiated that contains rules common for all unspecified customers.
The Rules Evaluator
This is the final part of the puzzle; the Rules Evaluator:
The Rules Evaluator is a stand-alone utility class that has one job only, satisfied by the EVALUATE() method – to parse the rules passed into it and return EX_STATE and EX_ABORT. It can be used for any collection of rules in a table of references to rules.
This is the current code for the evaluator:
METHOD zif_evaluator~evaluate. DATA: r_rule TYPE REF TO zif_rule. DATA: lv_state TYPE z_state, lv_state_overall TYPE z_state, lv_abort_overall TYPE boolean. CLEAR ex_state. lv_state_overall = c_pass. lv_abort_overall = '-'. ex_state = c_pass. LOOP AT im_rules INTO r_rule. REFRESH lt_status. r_rule->execute( IMPORTING ex_state = lv_state ex_abort = ex_abort ). * Only pass states can go to warning state IF lv_state_overall = zif_evaluator=>c_pass AND lv_state = zif_evaluator=>c_warn. lv_state_overall = zif_evaluator=>c_warn. ex_state = lv_state_overall. ENDIF. * Any state can go to error state IF lv_state = zif_evaluator=>c_fail. lv_state_overall = zif_evaluator=>c_fail. ex_state = lv_state_overall. ENDIF. * Any one rule raises abort flag, needs to be preserved IF ex_abort = 'X'. lv_abort_overall = 'X'. ENDIF. ENDLOOP. ENDMETHOD.
- Most of the logic deals with aggregation of the results of the collective evaluation of the individual rules.
- A warning state can be issued only if the current overall state of evaluation if the preceding evaluated rules is a pass state.
- Any state can go to an error state, but it is irreversible; no subsequent rule evaluation can change that state.
- An abort condition is preserved to pass back with the method parameters.
Putting it all Together
Having explained the components in detail, it will probably add clarity to see it in action.
Below, I have some code that is responsible for processing an inbound IDOC message. Part of that functionality is to create a sales order as part of a multi-step process of:
- Create Sales Order
- Create Delivery with reference to Sales Order
- Allocate batches to Delivery
- Post Goods Issue
The Sales Order is to be created provided that:
- One has not already been created (due to successful processing of this stage of IDOC handling in a previous handling of the IDOC)
- The customer is not in credit block
- Several other checks
So how does it get executed? At run time, the code that handles the IDOC builds the rules and then evaluates them as follows. First, the Rule Builder Factory is invoked, in order to build the correct set of rules for a particular customer:
* Set the rule builder CALL METHOD me->set_rule_builder( EXPORTING im_partner = im_partner ).
Which invokes the factory to create the correct rule builder for the particular partner:
method ZIF_ORDERS~SET_RULE_BUILDER. zcl_orders_rule_bld_fact=>make_zcl_rule_builder( EXPORTING im_partner = im_partner RECEIVING ret_zcl_rule_builder = rule_builder ). endmethod.
Then the rules are prepared and returned:
** Set up the sales order creation check rules CALL METHOD me->build_sales_order_rules( EXPORTING im_orders = me IMPORTING ex_order_check_rules = lt_order_check_rules ).
Which is implemented thus:
METHOD zif_orders~build_sales_order_rules. DATA: lt_order_check_rules TYPE z_orders_rules_t. rule_builder->build_order_rules( EXPORTING im_orders = im_orders ). rule_builder->get_order_rules( IMPORTING ex_order_check_rules = lt_order_check_rules ). ex_order_check_rules = lt_order_check_rules. ENDMETHOD.
Finally the rules are evaluated:
** Evaluate sales order creation check rules CALL METHOD me->evaluate_sales_order_rules( EXPORTING im_order_check_rules = lt_order_check_rules IMPORTING ex_state = lv_state ).
Here is the code in the method:
METHOD zif_orders~evaluate_sales_order_rules. DATA: lv_abort TYPE boolean. CALL METHOD zcl_evaluator=>zif_evaluator~evaluate EXPORTING im_rules = im_order_check_rules IMPORTING ex_state = ex_state ex_abort = lv_abort. IF lv_abort = 'X'. CALL METHOD me->abort. ENDIF. ENDMETHOD.
Overall, in the original code responsible for creating the sales order, disruption of the original code has been reduced to a few lines of code and comments, with a minimal addition to cyclomatic complexity:
* Set up the sales order creation check rules CALL METHOD me->build_sales_order_rules( EXPORTING im_orders = me IMPORTING ex_order_check_rules = lt_order_check_rules ). * Evaluate sales order creation check rules CALL METHOD me->evaluate_sales_order_rules( EXPORTING im_order_check_rules = lt_order_check_rules IMPORTING ex_state = lv_state ex_abort = abort_state ). * Terminate if abort state. CHECK abort_state NE c_true. * Create sales order iff creation check rules do not fail. IF lv_state NE zif_cssm_evaluator=>c_fail. CALL METHOD me->create_sales_order. ENDIF.
Note from the above:
- No complex logic in the program directly
- All decision making logic has been farmed out to the rules
- Further changes to the rules will not impact the complexity of the original code above
- The Rules Pattern allows a clear way of separating check logic into independent, isolated units.
- Code complexity is reduced; the original investment in the implementation of the pattern is offset over the lifetime of the code by the significant reduction in complexity.
- Comprehensive regression tests are easier to implement, since the scope of any changes are limited to a particular rule.
- Code maintainability is enhanced by virtue of each rule being isolated from other rules.
My thanks to Steve Smith for publishing the Rules Pattern, which was a module that he contributed to Pluralsight’s Design Patterns library.
You missed an important step. Shooting the analyst who didn't do his job describing the requirements in the first place 🙂
If only more programmers would spend time thinking about the design of their code, they'd use patterns like this. One problem I've encountered often is that the skill level of programmers is so low, they don't even realise that they should implement a new class to the interface. Instead they change the implementation. Which works lovely for their scenario... and screws up all the other scenarios...
I’d love to have all requirements up front, and a box of bullets for those who failed to provide them ?
Unfortunately it never happens that way, so all you can do is try to indemnify your solution against the eventuality. I work with a very smart Technologist (as he calls himself – he hates to be pigeonholed) who, whenever he embarks on a new implementation asks himself the question ‘what could go wrong with this implementation?’; he calls it a ‘pre-mortem’ rather than waiting for the solution to go wrong and then holding a ‘post-mortem’.
I guess for me this design is a consequence of that kind of ‘pre-mortem’ thinking.
Concerning your comment regarding skill levels and subsequent maintenance of the solution – we will be handing this solution over to a third party outsourcing partner in a few months, and yes I am really worried about correct support of the solution. I have thought about this a bit (another pre-mortem) and have managed to persuade management that after the handover, there will be an associated multiple choice questionnaire, and any ‘resource’ that does not get a high enough score in the test will not be allowed to maintain the solution. Re-certification will also happen on a periodic basis (with different sets of questions, just to be sure ? ).
Make sure the resource candidates take the test together, and there's no possibility of communicating with the outside world!
thanks for the nice presentation.
This rule framework reduces complexity for developers, but given a large number of rules, how to explain the system behavior to the end user?
I think a log containing the description and the evaluation result of each rule applied (e.g. "Check that referenced sales order exists" and "passed"/"failed") is needed. This list could be added to the IDoc status or displayed.
This would give feedback, providing a simple check for testing and improve business acceptance.
There was actually more to the implementation than I presented, but I cut out some stuff in an attempt to focus on the core concepts. For instance, each rule appends status records to an IDoc status table in order to see what rule failed and why; so in essence I had a variant of what you had suggested.
Concerning your comment: " how to explain the system behavior to the end user?" - I have struggled with, and am still struggling with how to communicate the behavior effectively. I think that this blog was an attempt to see how successfully I could communicate the concepts. Not sure if I did a very good job, but I will take any feedback in the comments and see if I can improve on that.
If i understand correctly the “Rules” pattern separates the business rules from the actual program. That way you ensure changes to the rules doesn’t result in changing the code of the program.
IMHO this is a perfect use case for BRF+, isn’t it? Or do you see any specific advantage of the pattern over BRF+?
I agree, this is a perfect use case for BRF+; there is usually more than one way to solve a problem. I guess this could be regarded as a light substitute for a business rules engine.
My comment about using BRF+ was based on the exchange between you & Jacques, regarding tracing the rules, illustrating the behavior to the end-user etc. I just wanted to know if there's any specific reason for using the pattern over BRF+.
Anyway i think i have got my answer.
The point about a design pattern is that it does not matter how you implement it. You have a generic interface, or you use BRF+, or the HANA rules framework or whatever they come up with next,
Either way you are still following the pattern, and what a good pattern it is.
At the moment I have a requirement that has to be implemented in two different countries and they want to do it in totally different ways - one by order type, one by a field in the customer master. Regardless in both cases it boils down to "yes/no" based on the sales order data.
And in both cases the rules will change in the future - a lot. So abstracting the rule follows the good old "open closed principle".
Hit the nail right on the head!
So which abstraction technique did you use? Fyi, i started my BRF+ journey with your book.
I have to work with a "Frankenstein's Monster" if you forgive the pun..
That is, more modern code uses BRF+, our head office created their own version of BRF+ long before the real BRF+ came along, there are Z configuration tables, complex conditional algorithms, and the actual IMG.tables referenced by Z code.
They are all rules, and need to be treated by the calling program in the exact same manner.
Moreover some give yes/no answers, some give a structure of values as a result.In the latter case you could say you are doing several rules at once
i.e. for sales organisation A the widget value is X
for sales organisation A the wudget value is Y
you could have one rule table for each, but when you have 15 return values one rule table seems more logical.
In any event the pattern is the same
So an interface to abstract the exact mechanics is cool and groovy. Moreover there will be something after BRF+ I am sure e.g. HANA rules or whatever, that can change under the hood of the interface.
By the way, BRF+ is a sort of rules builder all by itself, as one function can evaluate one of many rules or indeed sets of rules.
If you modularise your code and even better use objects your code becomes very readable.
Thanks for taking the time to read through the blog posting!
In my experience there is plenty of code out there that has been written in the manner in which I described; I have to maintain a lot of code that has had a legacy of many developers working on it, with a diverse approach to code implementation. Sometimes there are opportunities for improvement (and I include myself in that observation).
The example scenario described was a hypothetical context, within which to perform the main objective, which was to present information that I believed could be worth disseminating to others,
The point I was trying to make was that if you develop your code using good practice, modularisation and all the other various techniques out there that promote good coding, you should not end up with a rats nest of if's, thens and whatevers.
I always try to keep in mind the guy following me who has to maintain my code....
Completely agree with you on that one!
And with this pattern new rules are somehow magically implemented without making any changes in the code and without a transport? Wouldn't you still need some kind of a code change in a class, not to mention that CASE to use the right type? I'm confused...
This looks like a good old modularization in a new dress but maybe I'm missing something.
If i may answer. I would like to reiterate the “open-closed” principle Paul Hardy has mentioned in his comment.
The driver program must be “open” for changes/enhancements/extensions, but “closed” for modification. Something like the “modification-free” enhancements (in SAP terminology).
Indeed code change(s) is/are required. But the question is where should the change be made?
A well structured procedural program is much better than an ill written OO Program, i would give you that. Trust me i’ve seen the latter a lot.
OK, so basically the good old modularization then. Classes instead of INCLUDEs. Pattern-shmattern, pfst. 🙂
Seems like a common sense but I guess it's not as common.
Thanks for the observation; it’s an entirely bona-fide comment. I need to break this down into two areas of response.
I think that there is very rarely a black and white clear cut delineation of issues in any problem to be solved, and this is no different.
Firstly; technically, you are entirely correct that in the scenario that I presented, there will need to be some minor code changes made to:
However, as Suhas pointed out, the question for me would be: in which scenario do I invite minimum disruption? I would posit that making changes in one location where expected (the factory) might not be too disruptive, and certainly does not disrupt the original code.
Additionally I concur; a transport is needed for the new subclass and factory.
Secondly, it is possible to add new classes without even making changes to the factory, further aligning to the ‘open for extension’ principle; there is a function module – SEO_CLASS_GET_ALL_SUBS – that will return all subclasses for a given superclass:
If you write your factory to use this, it is possible to dynamically determine any new instance of a class that appears in the system and have an instance of it created automatically in the factory method.
At the end of the day, I had to make a tradeoff between how much information to include in the blog posting in order to convey the essence of the message, versus the risk of omitting something that may lead to obfuscation. This was my first ever blog on any social media platform, so a bit of fine tuning is no doubt in order.
Finally, apologies if I have posted something that is perceived to be misleading; I hope that the essence of the message came across with the objective of sharing something that I believed to be useful, intact and untarnished.
Thanks for the explanation! I think the blog is perfectly fine other than some slight exaggeration of the virtues. In any case, it's much better than the vast majority of the SCN blogs these days. Special thanks for sticking up with just one font and color. 🙂
It'd probably be helpful if you started with more dynamic option, as described in your comment, that does not require the factory changes. It'd just make more sense IMHO. Otherwise some readers might think if you still need to change the factory then it defies the purpose.
I agree though that development is rarely black and white. We need to know both the tools and when it's practical to use them.
Thanks for sharing!