рефакторинга вложенный переключатель на объекты

Question

Я хотел бы некоторые советы по рефакторингу следующего метода:

public boolean makeDecision(String group, int level, int primaryAmount, int secondaryAmount) 
{ 
    if (group.equals("A")) 
    { 
     switch (level) 
     { 
      case 0 : return primaryAmount > 10000;break; 
      case 1 : return primaryAmount > 20000;break; 
      default : return secondaryAmount > 30000; break; 
     } 
    } 
    else if (group.equals("B")) 
    { 
     switch (level) 
     { 
       case 0 : return primaryAmount > 40000;break; 
       case 1 : return primaryAmount > 50000;break; 
       default : return secondaryAmount > 60000; break; 
     } 

    } 
    else if (group.equals("C")) 
    { 
     switch(level) 
     { 
      case 0 : return primaryAmount > 70000;break; 
      case 1 : return primaryAmount > 80000;break; 
      default : return secondaryAmount > 90000; break; 
     } 

    } 
    return false; 
} 

То, что я хотел бы достичь:

• Разрешить код следовать открыто/закрыто поскольку во времени будет больше групп/уровней.
• Удалите дублирование в инструкции переключателя уровня.
• В идеале удалите оператор переключения верхнего уровня группы.

Answer 1

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

public class Group { 
    public static Group A = new Group(10000,20000,30000); 
    public static Group B = new Group(40000,50000,60000); 
    public static Group C = new Group(70000,80000,90000); 

    private int primaryMin; 
    private int primaryMid; 
    private int secondaryMax; 

    private Group(int min, int mid, int max) { 
     primaryMin = min; 
     primaryMid = mid; 
     secondaryMax = max; 
    } 

    public boolean getLevel(int level, int primaryAmount, int secondaryAmount) { 
     if (level == 0) 
     return primaryAmount > primaryMin; 
     else if (level == 1) 
     return primaryAmount > primaryMid; 
     else 
     return secondaryAmount > secondaryMax; 
    } 
} 

Итак, теперь вы можете уменьшить верхний уровень заявление до

public boolean makeDecision(Group group, int level, int primaryAmount, int secondaryAmount) { 
    return group.getLevel(level, primaryAmount, secondaryAmount); 
} 

Вы можете рассмотреть вопрос об использовании null object pattern для обработки неизвестной группы.

Если вы скажете, что уровни будут возрастать со временем, я бы подумал о том, чтобы сделать почти то же самое снова, чтобы ввести класс Level и нажать цепочку if/else там как еще один слой полиморфсима. Тогда это станет double dispatch pattern, поскольку сначала вы отправили бы по типу Group, а затем по типу Level. Это должно означать, что вы можете добавить новый код без необходимости изменять существующие.

Answer 2

Поскольку каждый случай вычисляется путем тривиального сравнения, вы также можете делать все сравнения в любом случае.

Так вот предложение:

boolean[] aSol = { primary > 10000, primary > 20000, secondary > 30000 }; 
boolean[] bSol = { primary > 40000, primary > 50000, secondary > 60000 }; 
boolean[] cSol = { primary > 70000, primary > 80000, secondary > 90000 }; 

level = Math.min(level, 2); 
return group.equals("A") ? aSol[level] : 
     group.equals("B") ? bSol[level] : 
     group.equals("C") ? cSol[level] : 
     false; 

Я думаю, что это довольно читаемым и ремонтопригодны.

Вот еще несколько иной формулировке:

boolean[][] result = { 
     { primary > 10000, primary > 20000, secondary > 30000 }, 
     { primary > 40000, primary > 50000, secondary > 60000 }, 
     { primary > 70000, primary > 80000, secondary > 90000 } }; 

int groupId = Arrays.asList("A", "B", "C").indexOf(group); 

if (groupId == -1) 
    return false; 

boolean[] groupResult = result[groupId]; 
return groupResult[Math.min(level, groupResult.length-1)]; 

Другим вариантом является создание интерфейса с помощью метода

makeDecision(int level, int primaryAmount, int secondaryAmount) 

, а затем заполнить Map<String, GroupDecision> с процедурами принятия,

groupMap.put("A", new GroupDecision() { ... }); 
groupMap.put("B", new GroupDecision() { ... }); 
groupMap.put("C", new GroupDecision() { ... }); 

и затем позвоните по телефону

return groupMap.get(group).makeDecision(level, primaryAmount, secondaryAmount); 

Этот подход, вероятно, является наиболее расширяемым и читаемым подходом.