Рефакторинг логики if / else - PullRequest
35 голосов
/ 26 мая 2010

У меня есть класс Java с методом из тысячи строк логики if / else, подобной этой:

if (userType == "admin") {
     if (age > 12) {
          if (location == "USA") {
               // do stuff
          } else if (location == "Mexico") {
               // do something slightly different than the US case
          }
     } else if (age < 12 && age > 4) {
          if (location == "USA") {
               // do something slightly different than the age > 12 US case
          } else if (location == "Mexico") {
               // do something slightly different
          }
     }
 } else if (userType == "student") {
     if (age > 12) {
          if (location == "USA") {
               // do stuff
          } else if (location == "Mexico") {
               // do something slightly different than the US case
          }
     } else if (age < 12 && age > 4) {
          if (location == "USA") {
               // do something slightly different than the age > 12 US case
          } else if (location == "Mexico") {
               // do something slightly different
          }
     }

Как мне преобразовать это в нечто более управляемое?

Ответы [ 14 ]

26 голосов
/ 26 мая 2010

Вы должны использовать Стратегии , возможно, реализованные в перечислении, например ::

enum UserType {
  ADMIN() {
    public void doStuff() {
      // do stuff the Admin way
    }
  },
  STUDENT {
    public void doStuff() {
      // do stuff the Student way
    }
  };

  public abstract void doStuff();
}

Поскольку структура кода внутри каждой внешней ветви if в вашем коде выглядит примерно одинаково, на следующем шаге рефакторинга вы можете выделить это дублирование, используя шаблонные методы . В качестве альтернативы вы также можете превратить Location (и, возможно, Age) в стратегию.

Обновление: в Java4, вы можете вручную реализовать typesafe enum и использовать простые старые подклассы для реализации различных стратегий.

12 голосов
/ 26 мая 2010

Первое, что я хотел бы сделать с этим кодом, это создать типы Admin и Student, оба из которых наследуют от базового типа User. Эти классы должны иметь метод doStuff(), в котором вы скрываете остальную часть этой логики.

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

9 голосов
/ 26 мая 2010

Тысячи? Возможно, вам нужен механизм правил. Слюни могут быть жизнеспособной альтернативой.

Или шаблон Command, который инкапсулирует всю логику «сделать что-то немного другое» для каждого случая Сохраните каждую Команду на карте, указав в качестве ключа возраст, местоположение и другие факторы. Найти команду, выполнить ее, и все готово. Красиво и чисто.

Карта может быть сохранена как конфигурация и прочитана при запуске. Вы можете добавить новую логику, добавив новые классы и перенастроив.

6 голосов
/ 26 мая 2010

Сначала - используйте перечисления для userType и location - затем вы можете использовать операторы switch (улучшает читабельность)

Второе - используйте больше методов.

Пример:

switch (userType) {
  case Admin: handleAdmin(); break;
  case Student: handleStudent(); break;
}

и позже

private void handleAdmin() {
  switch (location) {
    case USA: handleAdminInUSA(); break;
    case Mexico: handleAdminInMexico(); break;
  }
}

Далее, найдите дублирующий код и добавьте его в дополнительные методы.

EDIT

Если кто-то заставляет вас кодировать Java без перечислений (например, вы вынуждены использовать Java 1.4.2), используйте 'final static' вместо перечислений или сделайте что-то вроде:

  if (isAdmin(userType)) {
    handleAdmin(location, age);
  } else if (isStudent(userType)) {
    handleStudent(location, age));
  }

//...

private void handleAdmin(String location, int age) {
  if (isUSA(location)) {
    handleAdminInUSA(age);
  } else if (isUSA(location)) {
    handleAdminInMexico(age);
  }
}

//...

private void handleAdminInUSA(int age) {
  if (isOldEnough(age)) {
    handleAdminInUSAOldEnough();
  } else if (isChild(age)) {
    handleChildishAdminInUSA(); // ;-)
  } //...
}
2 голосов
/ 26 мая 2010

Если код в блоках вписывается в несколько стандартных шаблонов, я бы создал таблицу со столбцами (тип, местоположение, minAge, maxAge, action), где 'action' - это перечисление, указывающее, какой из нескольких типов обработки делать. В идеале эта таблица должна читаться из файла данных или храниться в SQL.

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

2 голосов
/ 26 мая 2010

Вы можете использовать шаблон цепочки ответственности.

Рефакторинг if-else операторов в классы с интерфейсом IUserController, например.

Инициализируйте вашу цепочку в списке, в дереве или в любой другой подходящей структуре данных и выполните нужные функции в этой цепочке. Вы можете использовать шаблон Builder для создания упомянутой структуры данных. Он похож на шаблон стратегии, но в шаблоне цепочки ответственности экземпляр в цепочке может вызывать связанные экземпляры.

Кроме того, вы можете смоделировать конкретную функциональность местоположения, используя шаблон стратегии. Надеюсь, поможет.

2 голосов
/ 26 мая 2010

Я бы, наверное, сначала проверил, можете ли вы параметризировать код doStuff и doS SimilarStuff.

2 голосов
/ 26 мая 2010

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

Если вы действительно можете различить условие по типу пользователя, вы можете как минимум разбить тело каждого условия на отдельную функцию. Так что вы проверяете на основе типа и вызываете соответствующую функцию, специфичную для этого типа. Более OO решение состоит в том, чтобы представить каждого пользователя как класс, а затем переопределить некоторый метод вычисления, чтобы вернуть значение, основанное на возрасте. Если вы не можете использовать классы, но, по крайней мере, можете использовать перечисления, тогда вы сможете сделать более приятный оператор switch для перечислений. Переключение на строки поступит только в Java 7.

Что меня беспокоит, так это ситуации наложения (например, два типа пользователей с некоторыми общими правилами и т. Д.). Если это так, то вам лучше представить данные в виде некоторого внешнего файла (например, таблицы), который вы будете читать и поддерживать, и ваш код будет по существу работать как драйвер, который выполняет соответствующий поиск в этих данных. задавать. Это общий подход для сложных бизнес-правил, поскольку никто не хочет использовать и поддерживать тонны кода.

1 голос
/ 26 мая 2010

Вам действительно нужно разбить эти случаи на методы объекта. Я предполагаю, что эти строки и числа извлекаются из базы данных. Вместо того чтобы использовать их в необработанном виде в гигантской вложенной условной логике, вам нужно использовать эти фрагменты данных для создания объектов, которые моделируют желаемые взаимодействия. Рассмотрим класс UserRole с подклассами StudentRole и AdminRole, класс Region с подклассами США и Мексики и класс AgeGroup с соответствующими разделенными подклассами.

Как только вы создадите эту объектно-ориентированную структуру, вы сможете использовать хорошо понятные шаблоны объектно-ориентированного проектирования для перефакторинга этой логики.

1 голос
/ 26 мая 2010

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

Недостатком является то, что вам понадобится какой-то способ преобразовать информацию о состоянии в разные экземпляры. Преимущество - более чистый способ добавления поведения без необходимости изменять иерархию наследования.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...