Как реорганизовать большую функцию со многими конструкциями if? - PullRequest
2 голосов
/ 29 марта 2012

У нас есть приложение A в качестве основного приложения.Теперь мы создаем из него приложение B, которое использует подмножество функций приложения A.

Приложение A остается таким же, как оно, тогда как приложение B использует только подмножество A

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

Итак, функция выглядит так (на самом деле она длиннее, это отрывок):

  class SomeClass {

    Data prepareData() {

     if (this.bothId==1 || this.appAid=2 /*or only relevant for appA*/) {       
        if(this.data==null) { /*appA*/
            appAdoSmth(); /*appA*/
        }
        boolean merge=false; /*appA*/
        if (this.data==null) { /*appA*/
          merge=appAanalyze(data); /*appA*/
        }
        bothPrepare(merge); 
    } else if (bothIsRelevant()) {
      if(appArelevant()) { /*appA*/
        data=appAprepare(); /*appA*/
      } else {
        data=prepareBoth(); 
      }     
      bothUpdateSomeValue();
    }

  }

Как бы вы это сделали?

Ответы [ 4 ]

2 голосов
/ 29 марта 2012

Другие ответы касаются общего вопроса о том, как рефакторинг кода.Они предлагают хороший совет, но я не думаю, что это то, что вы спрашиваете.

Я думаю, что вы спрашиваете о возможных рефакторингах кода в вашем вопросе.

Трудно датьответ, который обычно применим или даже специально применим.(Пример кода не является вашим реальным кодом, и немного трудно понять, что он на самом деле «означает»).

  • AndreasD предлагает один подход: сломать большое сложное вложенное if в отдельные методы.

  • Другой подход заключается в использовании шаблона проектирования Stragegy.Разделите код, специфичный для каждого приложения, на классы стратегии.Например:

    interface Strategy {
        Data prepareData();
    }
    
    class GeneralStrategy implements Strategy {
        Data prepareData() {
            // do general preparation
        }
    }
    
    class App1Strategy extends GeneralStrategy {
        Data prepareData() {
            // do app1-specific preparation
            super.prepareData();
            // do more app1-specific preparation
        }
    }
    

и т. Д.

1 голос
/ 29 марта 2012

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

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

Удачи.

0 голосов
/ 29 марта 2012

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

Data prepareData() {
  if (this.bothId==1 || this.appAid=2 ) {
    handleCase1();  // <- you'll find better names for the methods      
  } else if (bothIsRelevant()) {
    handleCase2();
  }
}

private void handleCase1() {
  if(this.data==null) {
    appAdoSmth(); 
  }
  boolean merge=false; 
  if (this.data==null) { 
    merge=appAanalyze(data); 
  }
  bothPrepare(merge); 
}

private handleCase2() {
  if(appArelevant()) { 
    data=appAprepare(); 
  } else {
    data=prepareBoth(); 
  }     
  bothUpdateSomeValue();
}

Это, конечно, не уменьшает число if / else, но делает "основной" метод простым.

0 голосов
/ 29 марта 2012

На вашем месте я бы подготовил отчет о покрытии для этого класса.(Например, http://ecobertura.johoop.de/ или http://www.eclemma.org/) Таким образом, Eclipse может показывать покрытые линии зеленым цветом, и это помогает вам идентифицировать случаи. С этой помощью намного легче отделить зеленые линии и использовать их в методах.

...