What design pattern should be used when similarly if-else grows?

Question

We have some code like:

public class ErrorCodeUtil {
    public static void handleErrorCode(String errorCode) {
        if (errorCode.equals("1")) {
            handleErrorCode1();
        } else if (errorCode.equals("2")) {
            handleErrorCode2();
        } else if (errorCode.equals("3")) {
            handleErrorCode3();
        } else {
            handleErrorCodeByDefault(errorCode);
        }
    }

    public static void logByErrorCode(String errorCode) {
        if (errorCode.equals("1")) {
            logErrorCode1();
        } else if (errorCode.equals("2")) {
            logErrorCode2();
        } else if (errorCode.equals("3")) {
            logErrorCode3();
        } else {
            logErrorCodeByDefault(errorCode);
        }
    }
    //... a lot of method about error code
}

As you see, we have a Util to handle all things about ErrorCode, and when we want to add a special logic to an error code, we have to change many method of that utils class.

As expected, the value of error code varies in large range(possibly "112345" or "error_code_001"). So what design pattern is proper for that case?

What happens in your `logErrorCode1()`, `logErrorCode2()`, etc. functions? Would It not be better to pass in the errorCode string? something like `log(errorCode)` and remove the need for separate functions? — Eamonn McEvoy, Oct 22 '15 at 10:47
@EamonnMcEvoy if we make something like `log(errorCode)`, then we will get a super large method which has a lot of `if-else`s. — Sayakiss, Oct 22 '15 at 10:51
you could create an interface `Error` with a handle and a log function. This interface could be implemented into several other classes `Error1`, `Error2`. Further on your `handleErrorCode` could look like this `handleErrorCode(Error error)` and you would just proceed by calling `error.handle()` or `error.log()`. — SomeJavaGuy, Oct 22 '15 at 10:52
See [Which Java Design Pattern fits best for if-else statement including loop?](https://stackoverflow.com/questions/33010710) — f_puras, Oct 22 '15 at 11:01

Konstantin Yovkov · Accepted Answer · 2015-10-22T11:29:34.293

I would implement a decision table.

The table would consist of a set of mappings between one or more Predicates as key and Function as a value. If a Predicate condition is met, then the corresponding Function is executed. If no Predicate condition is met, then a default Function should be executed. This can (easily) replace the humongous "if-else" statement and should be easier for maintenance.

How a Predicate should look like? It should take a String (in your case) and should return a boolean indicating whether a condition is met or no:

interface Predicate {
    public boolean test(String x);
}

In the decision table, you'd add (anonymous) implementations of this interface as keys.

Hint: If you are already on Java8, even better, there's a built-in Predicate<T> interface. But if you're not, then you can introduce a Predicate interface of your own. :-)

The Function for the decision table's values will be a similar interface. It may (or may not) use an input parameters and should return void. In Java8 this is called a Consumer, however in my example I'll stick to the Function naming:

interface Function<T> {
    void apply(T t);
}

By constructing pairs between Predicate as a key and Function<ErrorCodeUtil> as a value, we'll populate the decision table. When a Predicate condition is met, then we'll invoke the corresponding Function's .apply() method:

The decision table itself can be a simple Map<Predicate, Function<ErrorCodeUtil>>:

Map<Predicate, Function<ErrorCodeUtil>> decisionTable = new HashMap<>();

and you should populate it at construction time or whenever you wish (just before the handleErrorCode() method logic):

Predicate equalsOne = new Predicate() { 
    public void test(String x) {
         return "1".equals(x);
    }
};
Function<ErrorCodeUtil> actionOne = new Function<ErrorCodeUtil>() {
    public void apply(ErrorCodeUtil t) {
        t.handleErrorCode1();
    }
}

decisionTable.put(equalsOne, actionOne);

and so for the other "condition-action" pairs, including the default action (i.e. the last else statement) for which the Predicate will always return true.

Note that in Java8, those anonymous classes can be significantly reduced by just using lambdas.

Finally, your "if-elseif" statements would be re-factored to a simple loop:

for (Map.Entry<Predicate, Function<ErrorCodeUtil>> entry: decisionTable.entrySet()){
    Predicate condition = entry.getKey();
    Function<ErrorCodeUtil> action = entry.getValue();
    if (condition.test(errorCode)) {
        action.apply(this);
    }
}

So, everytime you add a new condition, you won't have to touch the handleErrorCode(String error) method, but you'll have to just introduce a new (anonymous) implementation of Predicate and Function and .put() it into the decision table.

Really thanks, I think that will result a very clean code for our cases! I can't wait to try it! — Sayakiss, Oct 22 '15 at 11:20
You're welcome. I find it very useful for huge if-else statements. And indeed the code gets cleaner and easier to be tested and maintained. :) — Konstantin Yovkov, Oct 22 '15 at 11:22
What's the advantage of using a map here? You seem to use a map as a hack to relate 2 values together because you aren't relying on key lookups at all... Also, why not taking advantage of codes as keys and associate a chain of responsibility instead? `errorHandlingService.handleErrorCode(someCode) -> handlers.get(errorCode).execute()` Here `handlers.get` returns a chain of responsibility or a pipeline to handle the appropriate code. — plalx, Oct 22 '15 at 18:07
@plalx, it's only an example to demonstrate the general idea behind decision tables. Implementation can be improved, I agree. — Konstantin Yovkov, Oct 23 '15 at 06:23
A HashMap is all you need ;) There's no need for complex predicates when you just want to match a specific string. Predicates are good in more complex cases where multiple rules may appy Under various conditions. — plalx, Oct 23 '15 at 11:52

score 2 · Answer 2 · answered Oct 22 '15 at 10:54

2

I'd use Enum in that case.

public enum ErrorCodeEnum {
    1 {

        @Override
        public void handleErrorCode() {
            //doSomething
        }
    },
    2 {

        @Override
        public void handleErrorCode() {
            //doSomething
        }
    };

  public abstract void handleErrorCode();

}

Then, having the error code in hands...

ErrorCodeEnum.valueOf("1").handleErrorCode();

PS: This is what I'd use to replace if-else statement, as you asked. But I'd use a Logger API for that specific problem (seems like you're logging erros).

answered Oct 22 '15 at 10:54

Lucas

129
9

That doesn't respect the Open-Closed Principle. – plalx Oct 22 '15 at 18:09
1

@plalx, the enum could easily implement an interface. – jaco0646 Oct 22 '15 at 22:38

score 1 · Answer 3 · answered Oct 23 '15 at 05:12

You can keep all errorcodes in a list in one class. And check if list contains errorcode or not. So this will reduce your if...else logic.

You have written different methods to handle error codes like handleErrorCode1(), handleErrorCode2() etc. Now if list contains desired error code then you can invoke these methods through java reflection.

score 0 · Answer 4 · answered Oct 22 '15 at 10:53

regarding logging of errors, if all that is required is matching a code with a message, then a text file with mapping of codes to messages is the right way. the text file may be properties:

1=Item not Found
2=Item not valid

that can be loaded to a java.util.Properties instance, it may be xml that can be loaded into DOM or HashMap

<errors>
  <error>
    <code>1</code>
    <msg>Item not Found</msg>
  </error>
  <error>
    <code>2</code>
   <msg>Item not Valid</msg>
  </error>
<errors>

one advantage of this approach is that it can be made to support i18n if you specify language code in the file name and then get user language code from your client

What design pattern should be used when similarly if-else grows?

4 Answers4