Какой лучший способ избежать слишком большого количества if / else-if из следующего фрагмента кода?

14

Я пытаюсь написать сервлет, который выполняет задачу на основе значения "action", переданного в качестве входных данных.

Вот образец которого

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
        */

     }
    }

Все, что мне нужно, это избежать слишком большого количества проверок if / else-if в моем коде для лучшей читабельности и лучшего обслуживания кода. Так что пробовал для других альтернатив, таких как

1. переключить регистр - все еще он делает слишком много проверок перед выполнением моего действия

2. отражение

я] одна главная вещь - безопасность - которая позволяет мне получать доступ даже к переменным и методам в классе, несмотря на его спецификатор доступа - я не уверен, что я мог бы использовать это в своем коде

II], а с другой стороны, это занимает больше времени, чем простые проверки if / else-if

Есть ли лучший подход или лучший дизайн, который кто-то мог бы предложить организовать вышеприведенный код лучше?

отредактированный

Я добавил ответ для приведенного выше фрагмента, учитывая ответ ниже .

Но все же, следующие классы "ExecutorA" и "ExecutorB" делают только несколько строк кода. Полезно ли добавлять их как класс, а не как метод? Пожалуйста, посоветуйте в этом отношении.

Том Тейлор
источник
1
Возможный дубликат подходов к проверке нескольких условий?
комар
2
Почему вы перегружаете один сервлет 9 различными действиями? Почему бы просто не отобразить каждое действие на отдельной странице, поддерживаемой другим сервлетом? Таким образом, выбор действия выполняется клиентом, а код вашего сервера просто фокусируется на обслуживании запроса клиента.
Maybe_Factor

Ответы:

13

Основываясь на предыдущем ответе, Java позволяет перечислениям иметь свойства, чтобы вы могли определить шаблон стратегии, что-то вроде

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

Тогда ваша Executor(стратегия) будет

public interface Executor {
    foo execute();
}

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

И все ваши if / else в вашем doPostметоде становятся чем-то вроде

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

Таким образом, вы можете даже использовать лямбды для исполнителей в перечислениях.

Дж. Пичардо
источник
хорошо сказано .. Но мне нужно небольшое пояснение .. Все мои действия action1 (), action2 () будут состоять из нескольких строк кода ... хорошо ли будет упаковать его в класс?
Том Тейлор,
4
Это не количество строк, которые должны убедить вас создавать конкретные классы / объекты, а тот факт, что они представляют различные варианты поведения. 1 идея / концепция = 1 логический объект.
mgoeminne
2
@RajasubaSubramanian Вы также можете использовать лямбду или ссылку на метод, если считаете, что класс слишком тяжелый. Executorявляется (или может быть) функциональным интерфейсом.
Халк
1
@ J.Pichardo Спасибо за обновление :) Так как я все еще в java7, я не мог использовать лямбда-выражения. Итак, следовал enum реализации шаблона стратегии, предложенного здесь на dzone.com/articles/strategy-pattern-implemented
Том Тейлор
1
@RajasubaSubramanian круто, я узнал что-то новое,
J. Pichardo
7

Вместо использования отражения используйте выделенный интерфейс.

т.е. вместо:

      /**
       * 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);  

использование

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

Реализует каждое из них для каждого действия, а затем:

 // 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());
    }
}

Конечно, это решение не самое длинное, поэтому вам, возможно, не придется идти до такой длины.

Walfrat
источник
Я думаю, что это должно быть, ProcessActionа ActionProcessне так ли?
Том Тейлор
1
Да, я это исправил.
Вальфрат
1
И, в более общем смысле, ответ «использовать ООП механизмы». Итак, здесь вы должны определить «ситуацию» и связанное с ней поведение. Другими словами, представляйте свою логику абстрактным объектом, а затем управляйте этим объектом вместо его базовых гаек и болтов.
mgoeminne
Кроме того, естественным расширением подхода, предложенного @Walfrat, будет предложение (абстрактной) фабрики, которая создает / возвращает правильный ProcessAction в зависимости от указанного параметра String.
mgoeminne
@mgoeminne Это звучит правильно
J. Pichardo
2

Используйте шаблон команд , для этого потребуется интерфейс команд примерно такой:

interface CommandInterface {
    CommandInterface execute();
}

Если Actionsони легкие и дешевые в сборке, используйте фабричный метод. Загрузите имена классов из файла свойств, который отображает actionName=classNameи использует простой фабричный метод для создания действий для выполнения.

    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;
}

Если Действия стоят дорого, используйте пул, такой как HashMap ; однако в большинстве случаев я бы предположил, что этого можно избежать в соответствии с принципом единой ответственности, делегируя дорогостоящий элемент некоторому заранее созданному общему пулу ресурсов, а не самим командам.

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

Затем они могут быть выполнены с

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

Это очень надежный и не связанный друг с другом подход, который применяет SRP, LSP и ISP принципов SOLID . Новые команды не изменяют код сопоставления команд. Команды просты в реализации. Их можно просто добавить в файл проекта и свойств. Команды должны быть входящими, и это делает их очень производительными.

Мартин Спамер
источник
1

Вы можете использовать объект на основе перечисления, чтобы уменьшить потребность в жестком кодировании строковых значений. Это сэкономит вам время и сделает код более удобным для чтения и расширения в будущем.

 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;
      }
  }
Каран
источник
1

Шаблон Factory Method - это то, на что я обращаю внимание, если вы ищете масштабируемый и менее обслуживаемый дизайн.

Шаблон Factory Method определяет интерфейс для создания объекта, но пусть подкласс решает, какой класс создать. Фабричный метод позволяет отложить создание экземпляров класса до подкласса.

 abstract class action {abstract doStuff(action)}

action1, action2 ........ actionN конкретная реализация с реализацией метода doStuff.

Просто позвони

    action.doStuff(actionN)

Поэтому в будущем, если будет введено больше действий, вам просто нужно добавить конкретный класс.

Анудж Сингх
источник
опечатка abstarct -> абстрактный в первой строке кода. Пожалуйста, отредактируйте. Кроме того, можете ли вы добавить немного больше кода, чтобы очистить этот пример, чтобы показать более прямо, как это отвечает на вопрос ОП?
Джей Элстон
0

Со ссылкой на @J. Pichardo answer: Я пишу приведенный выше фрагмент кода следующим образом

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();
  }
}
Том Тейлор
источник
Разве вы не создаете слишком много классов, если слишком много действий? У нас есть лучшая реализация.
Вайбхав Шарма