How can we improve the OO design between two interfaces

Question

In the below sample code, how can we get rid of if-else ladder of concrete classes viz. NoProcesssorFoundError & UnknownError. I could extract an interface between Event1 and Event2 and pass their processors to process functions as the process1 and process2 is almost same except inputs and its procs. However, how to get rid of the if-else ladder of error concrete classe

public class EventProcessor {

    public void onEvent1(Event1 event1) {
        val result = process1(event1);
        if(result instanceof NoProcesssorFoundError) {
            event1.setStatus(RETRYABLE);
            ...
        } else(result instanceof UnknownError) {
            event1.setStatus(FAILED);
            ...
        }
    }

    public void onEvent2(Event2 event2) {
        val result = process2(event2);
        if(result instanceof NoProcesssorFoundError) {
            event2.setStatus(REJECTED);
            ...
        } else(result instanceof UnknownError) {
            event2.setStatus(FAILED);
            ...
        }
    }

    private Error process1(Event1 event1) {
        Event1processors.process(event1)
        ....
        if(true) {
            return new NoProcessorFoundError();
        } else {
            return new UnknownError();
        }
    }

    private Error process2(Event2 event2) {
        Event2processors.process(event2)
        ....
        if(true) {
            return new NoProcessorFoundError();
        } else {
            return new UnknownError();
        }
    }

    abstract class Error {
        class NoProcessorFoundError extends Error {}
        class UnknownError extends Error {}
        ...
    }
}

Answer 1

In my opinion one good solution is the command pattern. You can check the next URL: https://refactoring.guru/design-patterns/command/java/example.

The code should be something like:

You have to create one abstract class to define the command functions.

Command.java

public abstract class Command {
   public Command(){}

   public Error execute(){
      return null;
   }

   public Error getError(){
       if(true) {
           return new NoProcessorFoundError();
       } else {
           return new UnknownError();
       }
   }
}

Then, you must to create the 2 processes to split the functionalities.

Process1.java

public class Process1 extends Command {
    private Eventinterface event;

    public Process1(Eventinterface event){
       this.event = event;
    }

    @Override
    public Error execute(){
        Event1processors.process(this.event);
        Error result = getError();
        if(result instanceof NoProcesssorFoundError) {
            event1.setStatus(RETRYABLE);
            ...
        } else(result instanceof UnknownError) {
            event1.setStatus(FAILED);
            ...
        }
    }
}

Process2.java

public class Process2 extends Command {
    private Eventinterface event;

    public Process2(Eventinterface event){
       this.event = event;
    }

    @Override
    public Error execute(){
        Event2processors.process(this.event);
        Error result = getError();
        if(result instanceof NoProcesssorFoundError) {
            event2.setStatus(REJECTED);
            ...
        } else(result instanceof UnknownError) {
            event2.setStatus(FAILED);
            ...
        }
    }
}

Then, to use this patter you have 2 options. The first option is use the Processes classes directly.

Main1.java

public static void main(String[] args) {
    Event1 event1 = new Event1();
    Command process1 = new Process1(event1);
    process1.execute();
        
    Event2 event2 = new Event2();
    Command process2 = new Process2(event2);
    process2.execute();
}

Or you can create one CommandExecutor and get the command process using one enumerator. For this case, I think it is not the best option because if you have a lot of processes you have to define too much events before to instantiate the CommandExecutor.

CommandExecutor.java

public class CommandExecutor {
    HashMap<CommandEnum, Command> commandStack = new HashMap();
    
    public CommandExecutor(EventInterface... events) {
        commandStack.put(CommandEnum.PROCESS1, new Process1(events[0]));
        commandStack.put(CommandEnum.PROCESS2, new Process2(events[1]));
    }
    
    public Command getCommand(CommandEnum commandEndum) {
        return commandStack.get(commandEndum);
    }
}

Finally you can use this executor like this main file.

Main2.java

public static void main(String[] args) {
    Event1 event1 = new Event1();
    Event2 event2 = new Event2();
    CommandExecutor commandExecutor = new CommandExecutor(event1, event2);
        
    Command command = commandExecutor.getCommand(CommandEnum.PROCESS1);

    command.execute();
}

To refactor the if-else of the Error you can implement one Visitor pattern. You have different choices to change this if-else in this link: https://www.baeldung.com/java-instanceof-alternatives.

Answer 2

you can use a bit of polymorphism and try something like this:

abstract class Error {
    public abstract Status status();

    class NoProcessorFoundError extends Error {
        public static NoProcessorFoundError retrayable() {
            return new NoProcessorFoundError(RETRYABLE);
        }
        public static NoProcessorFoundError redjected() {
            return new NoProcessorFoundError(REDJECTED);
        }
    
        private final Status status;
        
        private NoProcessorFoundError(Status status) {
            this.status = status;
        }
        
        @Override
        public Status status() {
            return status;
        }
    }
    
    class UnknownError extends Error {
        @Override
        public Status status() {
            return FAILED;
        }
    }
    ...
}

And this can be used like so:

public void onEvent1(Event1 event1) {
    val result = process1(event1);
    event1.setStatus(event1.status());
}

public void onEvent2(Event2 event2) {
    val result = process2(event2);
    event2.setStatus(event2.status());
}

private Error process1(Event1 event1) {
    Event1processors.process(event1)
    ....
    if(true) {
        return NoProcessorFoundError.retrayable();
    } else {
        return new UnknownError();
    }
}

private Error process2(Event2 event2) {
    Event2processors.process(event2)
    ....
    if(true) {
        return NoProcessorFoundError.redjected();
    } else {
        return new UnknownError();
   }
}

As a result onEvent1 and onEvent2 are very similar, they can even be extracted together in a common onEventmethod.