javaoopdesign-patternsclass-designooad

How can we improve the OO design between two interfaces


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 {}
        ...
    }
}

Solution

  • 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.