How to make complex conditions look nice and save

Question

In my java application I have a huge set of conditions which decides just one action. My question is how to make it look nice (I use NetBeans so I'd prefer solution that will not be broken by its code formatting function). I'd also like to have there as low amount of if/else statements as possible, because I think it will make it faster.

My original code was a mess, so I made an action diagram:. Take a copy if you want to play with it. Please keep in mind that the diagram is not perfect as to UML syntax, partly because I made it using google docs.

This is the code:

if (!config.get("checkForSpecials") || event.isNotSpecial()) {
    if (config.get("filterMode").equals("blacklist")) {
        if (!itemFilter.contains(event.getItem().getName())) {
            item.process();
        }
    } else if (config.get("filterMode").equals("whitelist")) {
        if (itemFilter.contains(event.getItem().getName())) {
            item.process();
        }
    } else {
        item.process();
    }
}

There are two things I don't like about it - the conditions are not too clear (especially when I unfold full method names and config strings), and the fact that the process method call is there three times.

Answer 1

Believe it or not, the diagram is not that complex:) There is no loop, and it is rather linear.

Here's a pseudo code that implements it

void action()

    if <sort out specials>
        if <is it special>
            return;

    if <check for unsafe items>
        if not <safe items list contains item>
            return;

    if <filter status = on>
        if < filter mode = whitelist>
            if not <item is on filter>
                return;
        else // black list
            if not <item is on filter>
                return;

    // finally!            
    [process item]

For really complex diagram, the answer is ... goto ...

Answer 2

Factoring booleans out and caching return values from method calls can help clarify code.

In addition, plotting all the outcomes on a logic table can help. I use this tool to help.

With the linked tool:

A: config.get("filterMode").equals("blacklist")
B: config.get("filterMode").equals("whitelist")
C: filterContainsName (see below)

The tool churns out:

(!A && !B) || (!A && C) || (A && !C)

Which leads to the code below (with a small tweak that replaces (!A && C) with (B && C)):

boolean filterContainsName = itemFilter.contains(event.getItem().getName());
boolean useBlacklist       = config.get("filterMode").equals("blacklist");
boolean useWhitelist       = config.get("filterMode").equals("whitelist");

if (!config.get("safeMode") || event.isSafe()) {
    if((!useBlackList && !useWhiteList) ||
       ( useWhiteList &&  filterContainsName) ||
       ( useBlackList && !filterContainsName)) {
        item.process();
    }
}

Answer 3

Use maps. The key of the map is the condition/case, the value is a single method class/anonymouse interface that contains the logic for that condition. Whenever you encounter a certain condition/case, you simply do a lookup in the map and execute the related function. This way you can even split up your logic-by-condition into seperate classes (if needed for sake of code beauty). As an added bonus you'll probably gain a performance bonus when the # of conditions > 10.

Answer 4

Looks good as it is to me. Perhaps you can isolate the valid conditions for calling item.process() to a method to make it more easier to understand.

if (!config.get("safeMode") || event.isSafe()) {
    if (isItemValidForProcess(config, itemFilter, event)) {
        item.process();
    }
}

boolean isItemValidForProcess(config, itemFilter, event) {
    String filterMode = config.get("filterMode");
    if (filterMode.equals("whitelist")) {
        return itemFilter.contains(event.getItem().getName());
    }
    if (filterMode.equals("blacklist")) {
        return !itemFilter.contains(event.getItem().getName());
    }
    return true;
}