Apa cara yang lebih baik untuk melarikan diri dari terlalu banyak if / else-if dari snipet kode berikut?

14

Saya mencoba untuk menulis servlet yang melakukan tugas berdasarkan nilai "action" yang diteruskan sebagai input.

Berikut adalah contohnya

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

     }
    }

Yang saya butuhkan adalah keluar dari terlalu banyak if / else-if memeriksa kode saya untuk keterbacaan yang lebih baik dan pemeliharaan kode yang lebih baik. Maka dicoba untuk alternatif lain seperti

1. sakelar kasus - masih tetap melakukan terlalu banyak pemeriksaan sebelum melakukan tindakan

2. refleksi

i] satu hal utama adalah keamanan - yang memungkinkan saya untuk mengakses bahkan variabel dan metode di dalam kelas meskipun ada specifier aksesnya - saya tidak yakin cuaca saya bisa menggunakannya dalam kode saya

ii] dan yang lainnya membutuhkan waktu lebih dari sekadar pemeriksaan if / else-if

Apakah ada pendekatan yang lebih baik atau desain yang lebih baik yang seseorang sarankan untuk mengatur kode di atas dengan cara yang lebih baik?

Diedit

Saya telah menambahkan jawaban untuk cuplikan di atas dengan mempertimbangkan jawaban di bawah ini .

Tapi tetap saja, kelas-kelas berikut "ExecutorA" dan "ExecutorB" hanya melakukan beberapa baris kode. Apakah ini praktik yang baik untuk menambahkan mereka sebagai kelas daripada menambahkannya sebagai metode? Mohon saran dalam hal ini.

Tom Taylor
sumber
1
Kemungkinan duplikat dari Pendekatan untuk memeriksa berbagai kondisi?
nyamuk
2
Mengapa Anda membebani satu servlet dengan 9 aksi berbeda? Mengapa tidak hanya memetakan setiap tindakan ke halaman yang berbeda, didukung oleh servlet yang berbeda? Dengan cara itu pemilihan tindakan dilakukan oleh klien dan kode server Anda hanya berfokus pada melayani permintaan klien.
Maybe_Factor

Jawaban:

13

Berdasarkan jawaban sebelumnya, Java memungkinkan enum memiliki properti sehingga Anda bisa menentukan pola strategi, seperti

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

Maka Executor(Strategi) Anda akan menjadi

public interface Executor {
    foo execute();
}

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

Dan semua if / else Anda dalam doPostmetode Anda menjadi sesuatu seperti

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

Dengan cara ini Anda bahkan bisa menggunakan lambdas untuk pelaksana di enum.

J. Pichardo
sumber
well said .. Tapi saya perlu klarifikasi kecil .. Semua action saya action1 (), action2 () akan menjadi beberapa baris kode .. apakah akan lebih baik untuk mengemasnya dalam kelas?
Tom Taylor
4
Ini bukan jumlah baris yang harus meyakinkan Anda untuk membuat kelas / objek tertentu, tetapi fakta bahwa mereka mewakili perilaku yang berbeda. 1 ide / konsep = 1 objek logis.
mgoeminne
2
@RajasubaSubramanian Anda juga bisa menggunakan lambda atau referensi metode jika Anda merasa kelas terlalu berat. Executoradalah (atau bisa) antarmuka fungsional.
Hulk
1
@ J.Pichardo Terima kasih atas pembaruannya :) Karena saya masih di java7 saya tidak bisa menggunakan ekspresi lambda .. Jadi, ikuti implementasi enum dari pola strategi yang disarankan di sini di dzone.com/articles/strategy-pattern-implemented
Tom Taylor
1
@RajasubaSubramanian keren, saya belajar sesuatu yang baru,
J. Pichardo
7

Alih-alih menggunakan refleksi, gunakan antarmuka khusus.

yaitu bukannya:

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

Menggunakan

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

Menerapkan masing-masing untuk setiap tindakan dan kemudian:

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

Tentu saja solusi ini bukan yang tertinggi, jadi Anda mungkin tidak perlu sampai sejauh itu.

Walfrat
sumber
Saya pikir itu harus ProcessActionbukannya ActionProcessbegitu ...?
Tom Taylor
1
Ya saya memperbaikinya.
Walfrat
1
Dan, lebih umum, jawabannya adalah "gunakan mekanisme OOP". Jadi, di sini, Anda harus menentukan kembali "situasi" dan perilaku yang terkait. Dengan kata lain, mewakili logika Anda dengan objek abstrak, dan kemudian memanipulasi objek ini alih-alih mur dan baut yang mendasarinya.
mgoeminne
Juga, perpanjangan alami dari pendekatan yang diusulkan oleh @Walfrat akan terdiri dalam mengusulkan pabrik (abstrak) yang membuat / mengembalikan ProcessAction yang tepat tergantung pada parameter String yang ditentukan.
mgoeminne
@mgoeminne Kedengarannya benar
J. Pichardo
2

Gunakan Pola Perintah , ini akan membutuhkan Command Interface sesuatu seperti ini:

interface CommandInterface {
    CommandInterface execute();
}

Jika Actionsyang ringan dan murah untuk membangun kemudian gunakan Metode Pabrik. Memuat nama kelas dari file properti yang memetakan actionName=classNamedan menggunakan metode pabrik sederhana untuk membangun tindakan untuk eksekusi.

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

Jika Actions mahal untuk dibangun maka gunakan pool, seperti HashMap ; namun dalam kebanyakan kasus saya menyarankan ini bisa dihindari berdasarkan Prinsip Tanggung Jawab Tunggal mendelegasikan elemen mahal ke beberapa kumpulan sumber daya bersama yang sudah dibangun sebelumnya daripada perintah itu sendiri.

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

Ini kemudian dapat dieksekusi dengan

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

Ini adalah pendekatan yang sangat kuat dan terpisah yang menerapkan SRP, LSP dan ISP dari prinsip-prinsip SOLID . Perintah baru tidak mengubah kode perintah mapper. Perintahnya sederhana untuk diimplementasikan. Mereka dapat ditambahkan ke file proyek dan properti. Perintah harus masuk kembali dan ini membuatnya sangat performant.

Martin Spamer
sumber
1

Anda dapat menggunakan objek berbasis enumerasi untuk mengurangi kebutuhan hardCoding nilai string. Ini akan menghemat waktu Anda dan membuat kode lebih rapi untuk dibaca & diperluas di masa depan.

 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
sumber
1

Pola Metode Pabrik adalah apa yang saya lihat jika Anda mencari desain yang skalabel dan kurang dapat dirawat.

Pola Metode Pabrik mendefinisikan antarmuka untuk membuat objek, tetapi biarkan subclass memutuskan kelas mana yang akan dipakai. Metode Pabrik memungkinkan instantiation penangguhan kelas untuk subkelas.

 abstract class action {abstract doStuff(action)}

action1, action2 ........ actionN implementasi konkret dengan mengimplementasikan doStuff Method untuk melakukan sesuatu.

Panggil saja

    action.doStuff(actionN)

Jadi di masa depan jika lebih banyak tindakan diperkenalkan, Anda hanya perlu menambahkan kelas konkret.

Anuj Singh
sumber
typo abstarct -> abstrak di baris kode pertama. Harap edit. Juga, dapatkah Anda menambahkan kode sedikit lebih banyak untuk menyiram contoh ini untuk menunjukkan secara lebih langsung bagaimana ini menjawab pertanyaan OP?
Jay Elston
0

Dengan mengacu pada @J. Pichardo menjawab saya menulis cuplikan di atas sebagai berikut

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
sumber
Bukankah Anda membuat terlalu banyak kelas jika ada terlalu banyak tindakan. Apakah kita memiliki implementasi yang lebih baik.
Vaibhav Sharma