¿Cuál es la mejor manera de escapar de demasiados if / else-if del siguiente fragmento de código?

14

Estoy tratando de escribir un servlet que realiza una tarea basada en el valor de "acción" que se le pasó como entrada.

Aquí está la muestra de que

public class SampleClass extends HttpServlet {
     public static void action1() throws Exception{
          //Do some actions
     }
     public static void action2() throws Exception{
          //Do some actions
     }
     //And goes on till action9


     public void doPost(HttpServletRequest req, HttpServletResponse res)throws ServletException, IOException {
          String action = req.getParameter("action");

          /**
           * I find it difficult in the following ways
           * 1. Too lengthy - was not comfortable to read
           * 2. Makes me fear that action1 would run quicker as it was in the top
           * and action9 would run with a bit delay - as it would cross check with all the above if & else if conditions
           */

          if("action1".equals(action)) {
               //do some 10 lines of action
          } else if("action2".equals(action)) {
               //do some action
          } else if("action3".equals(action)) {
               //do some action
          } else if("action4".equals(action)) {
               //do some action
          } else if("action5".equals(action)) {
               //do some action
          } else if("action6".equals(action)) {
               //do some action
          } else if("action7".equals(action)) {
               //do some action
          } else if("action8".equals(action)) {
               //do some action
          } else if("action9".equals(action)) {
               //do some action
          }

          /**
           * So, the next approach i tried it with switch
           * 1. Added each action as method and called those methods from the swith case statements
           */
          switch(action) {
          case "action1": action1();
               break;
          case "action2": action2();
               break;
          case "action3": action3();
               break;
          case "action4": action4();
               break;
          case "action5": action5();
               break;
          case "action6": action6();
               break;
          case "action7": action7();
               break;
          case "action8": action8();
               break;
          case "action9": action9();
               break;
          default:
               break;
          }

          /**
           * Still was not comfortable since i am doing un-necessary checks in one way or the other
           * So tried with [reflection][1] by invoking the action methods
           */
          Map<String, Method> methodMap = new HashMap<String, Method>();

        methodMap.put("action1", SampleClass.class.getMethod("action1"));
        methodMap.put("action2", SampleClass.class.getMethod("action2"));

        methodMap.get(action).invoke(null);  

       /**
        * But i am afraid of the following things while using reflection
        * 1. One is Security (Could any variable or methods despite its access specifier) - is reflection advised to use here?
        * 2. Reflection takes too much time than simple if else
        */

     }
    }

Todo lo que necesito es escapar de demasiados controles if / else-if en mi código para una mejor legibilidad y un mejor mantenimiento del código. Intenté otras alternativas como

1. cambio de mayúsculas y minúsculas : todavía hace demasiadas comprobaciones antes de realizar mi acción

2. reflexión

i] una cosa principal es la seguridad, que me permite acceder incluso a las variables y métodos dentro de la clase a pesar de su especificador de acceso, no estoy seguro de si podría usarlo en mi código

ii] y el otro es que lleva más tiempo que las simples comprobaciones if / else-if

¿Hay algún mejor enfoque o mejor diseño que alguien pueda sugerir para organizar el código anterior de una mejor manera?

EDITADO

He agregado la respuesta para el fragmento anterior considerando la respuesta a continuación .

Pero aún así, las siguientes clases "EjecutorA" y "EjecutorB" solo hacen unas pocas líneas de código. ¿Es una buena práctica agregarlos como clase que agregarlo como método? Por favor avise al respecto.

Tom Taylor
fuente
1
¿Posible duplicado de enfoques para verificar múltiples condiciones?
mosquito
2
¿Por qué está sobrecargando un solo servlet con 9 acciones diferentes? ¿Por qué no simplemente asignar cada acción a una página diferente, respaldada por un servlet diferente? De esa manera, el cliente realiza la selección de la acción y el código de su servidor solo se enfoca en atender la solicitud del cliente.
Maybe_Factor

Respuestas:

13

Según la respuesta anterior, Java permite que las enumeraciones tengan propiedades para que pueda definir un patrón de estrategia, algo así como

public enum Action {
    A ( () -> { //Lambda Sintax
        // Do A
       } ), 
    B ( () -> executeB() ), // Lambda with static method
    C (new ExecutorC()) //External Class 

    public Action(Executor e)
        this.executor = e;
    }

    //OPTIONAL DELEGATED METHOD
    public foo execute() {
        return executor.execute();
    }

    // Action Static Method
    private static foo executeB(){
    // Do B
    }
}

Entonces su Executor(Estrategia) sería

public interface Executor {
    foo execute();
}

public class ExecutorC implements Executor {
    public foo execute(){
        // Do C
    }
}

Y todos tus if / else en tu doPostmétodo se convierten en algo así

public void doPost(HttpServletRequest req, HttpServletResponse res) throws ServletException, IOException {
    String action = req.getParameter("action");
    Action.valueOf(action).execute();
}

De esta manera, incluso podría usar lambdas para los ejecutores en las enumeraciones.

J. Pichardo
fuente
bien dicho ... Pero necesito una pequeña aclaración ... Todas mis acciones action1 (), action2 () serían pocas líneas de código ... ¿será bueno empaquetarlo dentro de una clase?
Tom Taylor
44
Este no es el número de líneas que deberían convencerlo de crear clases / objetos específicos, sino el hecho de que representan comportamientos diferentes. 1 idea / concepto = 1 objeto lógico.
mgoeminne
2
@RajasubaSubramanian También podría usar una referencia lambda o método si considera que una clase es demasiado pesada. Executores (o puede ser) una interfaz funcional.
Hulk
1
@ J. Pichardo Gracias por la actualización :) Como todavía estoy en java7, no pude usar la expresión lambda. Entonces, seguí la implementación enum del patrón de estrategia sugerido aquí en dzone.com/articles/strategy-pattern-implemented
Tom Taylor
1
@RajasubaSubramanian cool, aprendí algo nuevo,
J. Pichardo
7

En lugar de usar la reflexión, use una interfaz dedicada.

es decir, en lugar de:

      /**
       * Still was not comfortable since i am doing un-necessary checks in one way or the other
       * So tried with [reflection][1] by invoking the action methods
       */
      Map<String, Method> methodMap = new HashMap<String, Method>();

    methodMap.put("action1", SampleClass.class.getMethod("action1"));
    methodMap.put("action2", SampleClass.class.getMethod("action2"));

    methodMap.get(action).invoke(null);  

Utilizar

 public interface ProcessAction{
       public void process(...);
 }

Implementa cada uno de ellos para cada acción y luego:

 // as attribute
Map<String, ProcessAction> methodMap = new HashMap<String, ProcessAction>();
// now you can add to the map you can either hardcode them in an init function
methodMap.put("action1",action1Process);

// but if you want some more flexibility you should isolate the map in a class dedicated :
// let's say ActionMapper and add them on init : 

public class Action1Manager{
    private static class ProcessAction1 implements ProcessAction{...}

    public Action1Manager(ActionMapper mapper){
       mapper.addNewAction("action1", new ProcessAction1());
    }
}

Por supuesto, esta solución no es la más ligera, por lo que es posible que no necesite llegar a esa longitud.

Walfrat
fuente
Creo que debe ser en ProcessActionlugar de ¿ ActionProcesses así que ...?
Tom Taylor
1
Sí, lo arreglé.
Walfrat
1
Y, de manera más general, la respuesta es "usar mecanismos de OOP". Entonces, aquí, debe reificar la "situación" y su comportamiento asociado. En otras palabras, representar su lógica con un objeto abstracto y luego manipular este objeto en lugar de sus tuercas y tornillos subyacentes.
mgoeminne
Además, una extensión natural del enfoque propuesto por @Walfrat consistiría en proponer una fábrica (abstracta) que cree / devuelva el ProcessAction correcto según el parámetro String especificado.
mgoeminne
@mgoeminne Eso suena bien
J. Pichardo
2

Use el patrón de comando , esto requerirá una interfaz de comando similar a esta:

interface CommandInterface {
    CommandInterface execute();
}

Si Actionsson livianos y económicos de construir, use un Método de fábrica. Cargue los nombres de clase de un archivo de propiedades que asigna actionName=classNamey use un método de fábrica simple para construir las acciones para la ejecución.

    public Invoker execute(final String targetActionName) {
        final String className = this.properties.getProperty(targetAction);
        final AbstractCommand targetAction = (AbstractCommand) Class.forName(className).newInstance();
        targetAction.execute();
    return this;
}

Si las acciones son caras de construir, utilice un grupo, como un HashMap ; sin embargo, en la mayoría de los casos, sugeriría que esto podría evitarse bajo el Principio de responsabilidad única que delega el elemento costoso en algún grupo de recursos comunes preconstruido en lugar de los comandos en sí.

    public class CommandMap extends HashMap<String, AbstractAction> { ... }

Estos se pueden ejecutar con

    public Invoker execute(final String targetActionName) {
        commandMap.get(targetActionName).execute();
        return this;
}

Este es un enfoque muy robusto y desacoplado que aplica SRP, LSP e ISP de los principios SOLID . Los nuevos comandos no cambian el código del mapeador de comandos. Los comandos son simples de implementar. Se pueden agregar al proyecto y al archivo de propiedades. Los comandos deben ser reentrantes y esto lo hace muy eficaz.

Martin Spamer
fuente
1

Puede utilizar el objeto basado en enumeración para reducir la necesidad de codificar los valores de cadena. Le ahorrará algo de tiempo y hará que el código sea mucho más fácil de leer y extender en el futuro.

 public static enum actionTypes {
      action1, action2, action3....
  }

  public void doPost {
      ...
      switch (actionTypes.valueOf(action)) {
          case action1: do-action1(); break;
          case action2: do-action2(); break;
          case action3: do-action3(); break;
      }
  }
Karan
fuente
1

El patrón Método de fábrica es lo que busco si buscas un diseño escalable y menos mantenible.

El patrón Método de fábrica define una interfaz para crear un objeto, pero permite que la subclase decida qué clase instanciar. El Método de Fábrica permite que una clase difiera la instanciación a la subclase

 abstract class action {abstract doStuff(action)}

action1, action2 ........ actionN implementación concreta con el método doStuff implementando cosas que hacer.

Solo llama

    action.doStuff(actionN)

Entonces, en el futuro, si se introducen más acciones, solo necesita agregar una clase concreta.

Anuj Singh
fuente
error tipográfico abstarct -> resumen en la primera línea de código. Por favor editar Además, ¿puede agregar un poco más de código para eliminar este ejemplo y mostrar más directamente cómo esto responde la pregunta del OP?
Jay Elston el
0

Con referencia a @J. Respuesta de Pichardo, estoy escribiendo la modificación del fragmento anterior como el siguiente

public class SampleClass extends HttpServlet {

public enum Action {
    A (new ExecutorA()),
    B (new ExecutorB())

    Executor executor;

    public Action(Executor e)
        this.executor = e;
    }

    //The delegate method
    public void execute() {
        return executor.execute();
    }
}

public foo Executor {
    foo execute();
}

public class ExecutorA implements Executor{
   public void execute() {
      //Do some action
   }
}

public class ExecutorB implements Executor{
   public void execute() {
      //Do some action
   }
}

public void doPost(HttpServletRequest req, HttpServletResponse res)throws ServletException, IOException {

  String action = req.getParameter("action"); 
  Action.valueOf(action).execute();
  }
}
Tom Taylor
fuente
¿No estás creando demasiadas clases si hay demasiadas acciones? ¿Tenemos una mejor implementación?
Vaibhav Sharma