How to simplify complicated business "IF" logic?

Question

What are the good ways to handle complicated business logic that from the first glance requires many nested if statements?

Example:

Discount Coupon. could be:

1a) Value discount
1b) Percentage discount

2a) Normal discount
2b) Progressive discount

3a) Requires access coupon
3b) Do not require access coupon

4a) Applied only to the customer who already bought before
4b) Applied to any customer

5a) Applied to customer only from countries (X,Y,…)

That requires code even more complicated then this:

if (discount.isPercentage) {
    if (discount.isNormal) {
        if (discount.requiresAccessCoupon) {
        } else {
        }
    } else if (discount.isProgressive) {
        if (discount.requiresAccessCoupon) {
        } else {
        }
    }
} else if (discount.isValue) {
    if (discount.isNormal) {
        if (discount.requiresAccessCoupon) {
        } else {
        }
    } else if (discount.isProgressive) {
        if (discount.requiresAccessCoupon) {
        } else {
        }
    }
} else if (discount.isXXX) {
    if (discount.isNormal) {
    } else if (discount.isProgressive) {
    }
}

Even if you replace IFs to switch/case it's still too complicated. What are the ways to make it readable, maintainable, more testable and easy to understand?

Answer 1

Good question. "Conditional Complexity" is a code smell. Polymorphism is your friend.

Conditional logic is innocent in its infancy, when it’s simple to understand and contained within a few lines of code. Unfortunately, it rarely ages well. You implement several new features and suddenly your conditional logic becomes complicated and expansive. [Joshua Kerevsky: Refactoring to Patterns]

One of the simplest things you can do to avoid nested if blocks is to learn to use Guard Clauses.

double getPayAmount() {
if (_isDead) return deadAmount();
if (_isSeparated) return separatedAmount();
if (_isRetired) return retiredAmount();
return normalPayAmount();
};  

The other thing I have found simplifies things pretty well, and which makes your code self-documenting, is Consolidating conditionals.

double disabilityAmount() {
    if (isNotEligableForDisability()) return 0;
    // compute the disability amount

Other valuable refactoring techniques associated with conditional expressions include Decompose Conditional, Replace Conditional with Visitor, and Reverse Conditional.

Answer 2

Specification pattern might be what you are looking for.

Summary:

In computer programming, the specification pattern is a particular software design pattern, whereby business logic can be recombined by chaining the business logic together using boolean logic.

Answer 3

I would write a generic state-machine that feeds on lists of things to compare.

Answer 4

The object oriented way of doing it is to have multiple discount classes implementing a common interface:

dicsount.apply(order)

Put the logic for determining whether the order qualifies for the discount within the discount classes.

Answer 5

You should really see

Clean Code Talks - Inheritance, Polymorphism, & Testing
by Miško Hevery

Google Tech Talks November 20, 2008

ABSTRACT

Is your code full of if statements? Switch statements? Do you have the same switch statement in various places? When you make changes do you find yourself making the same change to the same if/switch in several places? Did you ever forget one?

This talk will discuss approaches to using Object Oriented techniques to remove many of those conditionals. The result is cleaner, tighter, better designed code that's easier to test, understand and maintain.

Answer 6

Using guard clauses might help some.

Answer 7

FWIW, I have used Hamcrest very successfully for this sort of thing. I believe you could say that it implements the Specification Pattern, @Arnis talked about.

Answer 8

My first thought is that this is not testable, which leads me to a solution, in order to get it testable.

if (discount.isPercentage) {
  callFunctionOne(...);
} else if (discount.isValue) {
  callFunctionThree(...);
} else if (discount.isXXX) {
  callFunctionTwo(...);
}

Then you can have each nested if statement be a separate call. This way you can test them individually and when you test the large group you know that each individual one works.

Answer 9

Make methods that checks for a particular case.

bool IsValueNormalAndRequiresCoopon(Discount discount){...}

bool IsValueNormalAndRequiresCoupon(Discount discount){...}

etc

Once you start doing that it becomes easier to see where you can abstract out common logic between the choices. You can then go from there.

For complex decisions I often end up with a class that handles the possible states.