CODEWITHSUNDEEP
refactoringstructureif-statement
Intru
Intru

Reputation: 650

How to refactor multiple complex if-else branches

This problem is for a piece of software that is use by companies that organise activities. When an activity is changed (status, starting time, number of persons), some people need to be notified of this (by e-mail). The code that is responsible for this started out very simple: the body of the e-mail contained all the old values and new values of the information that was changed.

Over the years, many small rules were introduced. For example: if the new status is 'cancelled', the subject of the mail should be: "Activity cancelled" instead of "Activity changed". If there is no previous status (so the activity is new), and the current status is "final", then the subject should be "New Activity on [date]" and the body should contain a full overview (so no changes).

These rules above just serve to illustrate the problem. There are quite a few more of them (concerning combinations of status/date/time/etc.), totalling up to about 500 lines of code.

The problem that occurs is that this code is currently quite hard to understand and maintain. New rules are introduced every now and then, and adding them without breaking other rules can be a pain. What would be the best way to rewrite code like this into more understandable and maintainable code? Currently, the order of the if-else branches is also really important. The first if statement is the most important, the next else-if statement is bit less important, until the final else clause for the most general case.

Upvotes: 3

Views: 559

Answers (2)

Jeff Foster
Jeff Foster

Reputation: 44706

Sounds like something a rules engine could help with. Drools is one option, but it's almost certainly complete overkill!

Do you have an interface representing a rule? Something like

public class Activity {}

public interface Rule {
     boolean applies(Activity);

     Activity applyRule(Activity x);
}

Given this, you could implement a reduce function to apply the rules in order until you reach the end of the list.

Activity applyRules(List<Rule> rules, Activity);

Upvotes: 4

James
James

Reputation: 9975

I would separate the rules and the application of them, so your code becomes

For all Rules
   if ThisRule.Applies(activity) then
       ThisRule.ApplyActions(email);
       break;

The rules would then be a class where each rule has Applies and ApplyActions or ConstructEmail or something.

The you construct a list of rules in a priority order. Once that's done you would only need to iterate over the list.

The main point of this is that Rules don't need to know about or care about other rules, all rules are effectively independent they only need to know if they are the rule for the activity given to them and if they then follow the actions for that rule.

The difficulty is then splitting out the different rules, e.g if you have a rule where the new status is cancelled so you must put cancelled in the header but you have another rule which also applies then you need a mechanism for applying multiple rules such as:

For all Rules
   if ThisRule.Applies(activity) then
       ThisRule.ApplyActions(email);
       if ThisRule.TerminalRule then
           break;

so that simple rules like changing the subject allow other rules to be applied as well.

Upvotes: 2

Related Questions