CODEWITHSUNDEEP
javaif-statement
StatelessDev
StatelessDev

Reputation: 294

Cleaner way of dealing with these if clauses?

I am wondering what would be a cleaner way of dealing with this bunch of if clauses. As an aditional information, it's an exception handler to be used in a Spark Framework study project.

This piece of code does its job and it's readable, but a possible further extension would convert it into a monster. Do you have any suggestion to refactor it?

    public ExceptionHandler<? super java.lang.Exception> handler() {
        return (e, request, response) -> {
            ErrorMessage error;

            if (e instanceof SomeException1) {
                response.status(422);
                error = new ErrorMessage("message1", e.getMessage());
            } else if (e instanceof SomeException2) {
                response.status(404);
                error = new ErrorMessage("message2", e.getMessage());
            } else if (e instanceof SomeException3) {
                response.status(500);
                error = new ErrorMessage("message3", e.getMessage());
            } else {
                response.status(500);
                error = new ErrorMessage("message4", e.getMessage());
            }

            [...]
        };
    }

Edit for clarification:

This piece of code is part of an exception handler method which is registered in the application's main class using Spark's exception() method. Something like this:

public class MainClass {

    public static void main(String[] args) {
      //register the exception handler
      exception(Exception.class, errorsHandler.handler()); //handler() is the method showed above.
    }    
}

Upvotes: 1

Views: 94

Answers (2)

RealSkeptic
RealSkeptic

Reputation: 34608

My version would be something like this:

class ExceptionInfo {
   Class<? extends Exception> cls;
   int errCode;
   String message;

   public ExceptionInfo(Class<? extends Exception> cls, int errCode, String message) {
       this.cls = cls;
       this.errCode = errCode;
       this.message = message;
   }
}

// Note that the last item is Exception.class, to serve as a default.

final static List<ExceptionInfo> EXCEPTION_LIST = Arrays.asList(
    new ExceptionInfo( SomeException1.class, 422, "message1"),
    new ExceptionInfo( SomeException2.class, 404, "message2"),
    new ExceptionInfo( SomeException3.class, 500, "message3"),
    new ExceptionInfo( Exception.class, 500, "message4" )
);

ExceptionInfo searchException( Exception e ) {
    return EXCEPTION_LIST.stream()
        .filter( info -> info.cls.isInstance(e) )
        .findFirst()
        .orElseThrow( IllegalStateException::new );
}

With this, you can get the ExceptionInfo compatible with the given e, and then use its errCode and message. This is fully compatible with instanceOf. Of course, you can use getters instead of accessing the fields of this mini-class directly.

Upvotes: 1

Sebastian
Sebastian

Reputation: 1722

A good way to avoid long if-/else chains is to use a mapping table. You could use a Map, that maps the Exception class to the status code. Like this:

private static Map<Class<?>, Integer> errorCodeMapping = new HashMap<>();

{
    errorCodeMapping.put(SomeException.class, 422);
    ...
}

Your exception handler would then set the code like

response.status(errorCodeMapping.get((e.getClass()));

The same can be done for a exception class to error message mapping. This way you could also provide a setter for new mappings, that have not to be hard coded into the Handler class itself.

Upvotes: 4

Related Questions