Turn Your Spaghetti Code into Functions, Part 2

Read Part 1, first.

In Part 1, we started with an example of common business logic code, and an analogy based on cramming an elephant into a Smart Car. We did some refactoring to untangle the nested if/else blocks, but we left after we finished cramming the elephant into a Smart Car.

In many ways, it feels "good enough", but what if I told you we can get it better? Java 8 brings us a new tool to contain and use the logic within an if statement - the Predicate. In terms of elephants driving cars, we can get it driving a stylish convertible.

So you have a lot of conditional logic, and you find yourself copy-pasting conditions from one logic block to the other. It's easy, it's seductive, but it's wrong. Copy pasting is error prone and extra work! If you're like me, you try to work as little as possible - that's the computer's job. Java 8 provides a new tool to prevent copy-pasting and keep your code DRY.

Using Predicates allows us to encapsulate logic as a variable. This makes for two features that are particularly powerful

1. Variable names communicate the intent of the logic
2. Logic is resusable and individually testable

And Now, With Predicates

Before Java 8, I wasn't tuned in to the functional programming world. The last time I remember hearing about Predicates was in school when I was ignoring a grammar lesson. It turns out that, when programming, Predicates can really improve your code.

Here we will make the if/else blocks more readable by creating Predicates out of the logic they represent. Predicates can be named, which allows developers to name rules in a way that allows for clear discussions with even non-technical users.

static final Predicate<WidgetTransfer> suffientAmount = trans -> trans.getTransferer().getAccount(trans.getFromAccount()).getBalance().compareTo(trans.getAmount()) > 0;

static final Predicate<String> isPartner = ttc -> ttc.equals("200");

static final Predicate<String> isFriendsAndFamily = ttc -> ttc.equals("710");

static final Predicate<String> isFriendAndFamilyDiscountLegal = ac -> ac.matches("574|213|363|510");

static final Predicate<String> isPartneringArea = ac -> ac.matches("907|412|213");

static final Predicate<String> isDirigibleArea = ac -> ac.matches("213");

static final Predicate<String> isDirigibleCategory = cat -> cat.equals("D");

static final Predicate<String> isInternal = tc -> tc.equals("I");

public static final String checkWidgetTransfer(WidgetTransfer transfer) {

    String businessRuleErrors = "";

String transferTypeCode = transfer.getTransferTypeCode();
String areaCode = transfer.getAreaCode();
String category = transfer.getTransferer().getCategory();
String typeCode = transfer.getTypeCode();

if (suffientAmount.test(transfer)) {
    businessRuleErrors += "Insufficient balance to transfer ; ";
}

if (isPartner.test(transferTypeCode)
        &amp;&amp; isPartneringArea.negate().test(areaCode)) {
    businessRuleErrors += "This area is not a transfer eligible area. ; ";
}

if (isPartner.test(transferTypeCode)
        &amp;&amp; isDirigibleArea.test(areaCode)
        &amp;&amp; isDirigibleCategory.test(category)) {
    businessRuleErrors += "D Category Transferer can only be transferred in transfer area 213. ; ";
}

if (isFriendsAndFamily.test(transferTypeCode)
        &amp;&amp; isFriendAndFamilyDiscountLegal.negate().test(areaCode)) {
    businessRuleErrors += "This area is not an eligible area. ; ";

}

if (isInternal.negate().test(typeCode)
        &amp;&amp; !isBlockSize(transfer)) {
    businessRuleErrors += "Amount is too small for I type transfer. ; ";
}

if (isInternal.negate().test(typeCode)
        &amp;&amp; isTotalOverCap(transfer)) {
    businessRuleErrors += "This transfer is too large. ; ";
}

return businessRuleErrors;



    

    




}

The Good

• Each if block is readable in something approximating "business English"
• The rules are defined as Predicates
  • The rules are portable and reusable.
  • The rules are also individually testable, without having to test each branch at once

The Bad

• This technique still uses && which is not idiomatic with functions in Java
  • We are forced to use && because the Predicates take different types of objects, so we can't chain them together
• While the Predicates that make up the rules are portable, the rules themselves are made of multiple Predicates and are not portable
• Nothing has been done that can't be done with ordinary methods
  • Good old public boolean isSufficientAmount(String amount) would suffice
• We still have to create all these property variables to get the appropriate values to give to our Predicates

Predicate Chaining

Let's fix some of the stuff on the 'bad' list from the previous example.

We can get rid of && by using just a little bit more of the functional interface and refactoring the Predicates to all take the same type of object, in this case a WidgetTransfer object. The goal is to make our Predicates like legos - interlocking and interchangeable.

static final Predicate<WidgetTransfer> suffientAmount = trans -> trans.getTransferer().getAccount(trans.getFromAccount()).getBalance().compareTo(trans.getAmount()) > 0;

static final Predicate<WidgetTransfer> isPartner = trans -> trans.getTransferTypeCode().equals("200");

static final Predicate<WidgetTransfer> isFriendsAndFamily = trans -> trans.getTransferTypeCode().equals("710");

static final Predicate<WidgetTransfer> isFriendAndFamilyDiscountLegal = trans -> trans.getAreaCode().matches("574|213|363|510");

static final Predicate<WidgetTransfer> isPartneringArea = trans -> trans.getAreaCode().matches("907|412|213");

static final Predicate<WidgetTransfer> isDirigibleArea = trans -> trans.getAreaCode().matches("213");

static final Predicate<WidgetTransfer> isDirigibleCategory = trans -> trans.getTransferer().getCategory().equals("D");

static final Predicate<WidgetTransfer> isInternal = trans -> trans.getTypeCode().equals("I");

static final Predicate<WidgetTransfer> isBlockSize = trans -> isBlockSize(trans);

static final Predicate<WidgetTransfer> isTotalOverCap = trans -> isTotalOverCap(trans);

public static final String checkWidgetTransfer(WidgetTransfer transfer) {

    String businessRuleErrors = "";

if (suffientAmount.test(transfer)) {
    businessRuleErrors += "Insufficient balance to transfer ; ";
}

if (isPartner.and(isPartneringArea.negate()).test(transfer)) {
    businessRuleErrors += "This area is not a transfer eligible area. ; ";
}

if (isPartner.and(isDirigibleArea).and(isDirigibleCategory).test(transfer)) {
    businessRuleErrors += "D Category Transferer can only be transferred in transfer area 213. ; ";
}

if (isFriendsAndFamily.and(isFriendAndFamilyDiscountLegal.negate()).test(transfer)) {
    businessRuleErrors += "This area is not an eligible area. ; ";
}

if (isInternal.negate().and(isBlockSize.negate()).test(transfer)) {
    businessRuleErrors += "Amount is too small for I type transfer. ; ";
}

if (isInternal.negate().and(isTotalOverCap.negate()).test(transfer)) {
    businessRuleErrors += "This transfer is too large. ; ";
}

return businessRuleErrors;



    

    




}

The Good

• We get rid of extra variables that hold string values from the WidgetTransfer object
• We compact our if-blocks while retaining readability
• We only evaluate one type of object

The Bad

There is very little bad about this refactor point. The conditionals are all very easy to read. It's clear what each rule is, and what branch. If I didn't know what I have planned for the next article, I would be satisfied to stop here.

Next Steps

All our rules are Predicates and each rule takes the same kind of object, a WidgetTransfer. That makes our rule composable in the fashion demonstrated above, but there are some improvements we can make to how we compose the business rules.

The first improvement is to combine small rules into larger rules - we are doing this in conditional statements, but it is just as easy to do so in a Predicate. We can also create a Validator object to create a collection of business rules and error messages. A Validator dispenses with the need to create a complex nested if/else logic structure, and is a concrete unit of business logic that can be shared, re-used and tested.

Sign up for my email list to get notifcations about updates in continuing series, and a monthly digest of interesting programming and tech leadership articles

We will go over using Validators in the to-be-written Turn Your Spaghetti Code into Functions, Part III

Credits

Thank you roebot for the left facing Smart Car

Thank you Oliver Dodd for the elephant

Thank you Phillip Pessar for the convertible

Thank you JPL and NASA/Space Telescope Science Institute for the edge-on galaxy picture

Thank you Philip Sheldrak for the grammar school

Comments

[Gunnar Gissel]

Does anyone else really like the unicorn reaction? I'm not really sure what it means, but I like seeing the thing

[Dam Ur]

Nice article !

I generally prefer to modify the class itself instead of having predicates like these. For example :

trans.isPartnerTypeCode()

instead of

trans.getTransferTypeCode().equals("200")

The cons I see when using the latter is that:

• you expose the type with which the code is stored (String)
• you expose the value of the partner type code.

I prefer to encapsulate/hide this kind of things inside the class WidgetTransfer. If the type of the type code property, or the value of a partner type code change someday, this won't impact code using this class. Here you will have to keep this checker class up-to-date.

But this approach may have other drawbacks...

[Gunnar Gissel]

I'm going to lead with saying that this is an intermediate step - I'm going somewhere with the predicates that makes testing business rules really simple. The specific pain I'm trying to deal with is business rule tests that take a lot of setup and business object mutation to trigger a particular branch of business logic.

That said, your approach is good, and meshes well with OO domain driven design. I've used it myself, when starting from scratch. My big complaint is that it is hard to retrofit on something that was built without really following OO or DDD principles.

In this example, I'm assuming a fairly large body of pre-existing code. I'm loosely basing it off one of the code bases I work with. In that particular case, we have many POJOs that have getters and setters and little to nothing else. Things are not well encapsulated and data has leaked everywhere. The leakiness has made changing types or values of codes impractical. In my particular case the codes are also widely shared between several government and industry bodies and can't change without causing a riot. I recognize that's not ideal design, but the bones of it were started when I was in elementary school, so it is what it is...

We also have very large business rule utility classes that operate on these objects. Sometimes the business rules even mutate the objects they are validating! O_o

I rather like a functional approach for cleaning up that situation because the cat is already out of the bag as far as exposing types and values. We have sooo much code that hard codes something like "200" where it checks for values. If leaky abstractions were toxic waste, we'd be on a superfund site.

As a separate effort, rooting out these hard coded values is good. I like to replace them with enums, but sometimes other approaches are useful. I've been moderately happy with public static final String variables, or with new classes that encapsulate some concept that wasn't discovered when the initial class was written.

What I'm exploring here is how Java 8's Functional interface might help with refactoring code that was never very OO to begin with. Frankly, a lot of the traditional 'enterprisey' OO Java code is pretty hard to follow for me, so I don't feel like I'm losing much. I'm not completely sure where this investigation will end up, or if it will be 'better', but here's the general outline I see:

• Basic POJOs
• That implement a lot of small interfaces
• Handled by functions
• That are composed into rules

I don't really talk about the POJOs or the small interfaces thing here - that has to do with this dislike of inheritance I've developed over the years. Maybe another post on another day. I also haven't really gotten into composition of functions as rules - that's the next post.