Метод извлечения с продолжением

Question

Мы реорганизуем длинный метод; он содержит длинный цикл for со многими операциями continue. Я хотел бы просто использовать рефакторинг Extract Method, но автоматизированный Eclipse не знает, как обрабатывать условное ветвление. Я тоже.

Наша текущая стратегия ввести keepGoing флаг (переменную экземпляра, так как мы собираемся хотеть метод экстракта), установите его в ложное в верхней части петли, и заменить каждый продолжить с установкой флага к true, а затем обертывание всего следующего материала (на разных уровнях вложенности) внутри предложения if (keepGoing). Затем выполните различные экстракции, затем замените назначения keepGoing с ранней отдачей от извлеченных методов, затем избавьтесь от флага.

Есть ли лучший способ?

Update: В ответ на замечания - я не могу разделить код, но вот анонимный отрывок:

private static void foo(C1 a, C2 b, C3 c, List<C2> list, boolean flag1) throws Exception { 
    for (int i = 0; i < 1; i++) { 
     C4 d = null; 
     Integer e = null; 
     boolean flag2 = false; 
     boolean flag3 = findFlag3(a, c); 
     blahblahblah(); 
     if (e == null) { 
      if (flag1) { 
       if (test1(c)) { 
        if (test2(a, c)) { 
         Integer f = getF1(b, c); 
         if (f != null) 
          e = getE1(a, f); 
         if (e == null) { 
          if (d == null) { 
           list.add(b); 
           continue; 
          } 
          e = findE(d); 
         } 
        } else { 
         Integer f = getF2(b, c); 
         if (f != null) 
          e = getE2(a, f); 
         if (e == null) { 
          if (d == null) { 
           list.add(b); 
           continue; 
          } 
          e = findE(d); 
         } 
         flag2 = true; 
        } 
       } else { 
        if (test3(a, c)) { 
         Integer f = getF2(b, c); 
         if (f != null) 
          e = getE2(a, f); 
         if (e == null) { 
          if (d == null) { 
           list.add(b); 
           continue; 
          } 
          e = findE(d); 
         } 
         flag2 = true; 
        } else { 
         if (d == null) { 
          list.add(b); 
          continue; 
         } 
         e = findE(d); 
         flag2 = true; 
        } 
       } 
      } 
      if (!flag1) { 
       if (d == null) { 
        list.add(b); 
        continue; 
       } 
       e = findE(d); 
      } 
     } 
     if (e == null) { 
      list.add(b); 
      continue; 
     } 
     List<C2> list2 = blahblahblah(b, list, flag1); 
     if (list2.size() != 0 && flag1) { 
      blahblahblah(); 
      if (!otherTest()) { 
       if (yetAnotherTest()) { 
        list.add(b); 
        continue; 
       } 
       blahblahblah(); 
      } 
     } 
    } 
} 

Answer 1

Это один из тех веселых, где ни один образец не доставит вас туда.

Я буду работать с ним итеративно.

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

Далее я думаю, что я возьму некоторые внутренние куски и посмотрю, не могут ли они быть извлечены в отдельный метод. Похоже, что первые два больших блока (внутри «if (test2 (a, c)) {« и его другое выражение) очень похожи. Существует логика вырезания и вставки, которая должна быть одинаковой.

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

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

Редактировать (После просмотра немного больше): Существует так много, что было бы очень интересно пройти. Помните, что когда вы закончите, вы не хотите дублирования кода - и я уверен, что все это можно было бы написать без единого, если ... я думаю, что все ваши ifs - это случаи, которые могут/должны быть легко обработаны полиморфизмом.

О, и как ответ на ваш вопрос о затмении, не желая этого делать - даже не пытайтесь автоматизировать рефакторинг с этим, просто сделайте это вручную. Материал внутри этого первого, если() нужно вытащить в метод, потому что он практически идентичен предложению в else()!

Когда я делаю что-то подобное, я обычно создаю новый метод, переместите код из if в новый метод (оставив только вызов нового метода внутри if), затем запустите тест и убедитесь, что вы ничего не сломал.

затем перейдите по строкам и проверьте, нет ли разницы между кодом if и его else. Если есть, возместите его, передав разницу в качестве новой переменной методу. После того, как вы уверены, что все одинаково, замените предложение else вызовом. Повторите тест. Скорее всего, в этот момент станет очевидным несколько дополнительных оптимизаций, вы, скорее всего, потеряете все, если объединив свою логику с переменной, которую вы передали, чтобы различать два вызова.

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

Answer 2

continue в основном аналог раннего возвращения, не так ли?

for (...) { 
    doSomething(...); 
} 

private void doSomething(...) { 
    ... 
    if (...) 
     return; // was "continue;" 
    ... 
    if (!doSomethingElse(...)) 
     return; 
    ... 
} 

private boolean doSomethingElse(...) { 
    ... 
    if (...) 
     return false; // was a continue from a nested operation 
    ... 
    return true; 
} 

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

Answer 3

Если бы я столкнулся с вашей ситуацией, я бы посмотрел на использование других методов рефакторинга, таких как «заменить условный на полиморфизм». Тем не менее, вы всегда должны сделать одну вещь за один раз, так что если вы первый хотите извлечь метод у вас есть два варианта:

1. Добавить в «keepGoing» флаг
2. Бросить исключение из метода

Из этих двух вариантов, я думаю, флаг keepGoing лучше. Я бы не остановил рефакторинг после того, как вы извлекли этот метод. Я уверен, что если у вас есть меньший метод, вы найдете способ удалить этот флаг и иметь более чистую логику.

Answer 4

Я собираюсь обобщить ответы здесь, приняв ответ Билла К как наиболее полный. Но у каждого было что-то хорошее, и я мог бы использовать любой из этих подходов в следующий раз, когда я столкнусь с такой ситуацией.

---

mmyers: Вырезать из тела цикла, вставить его в новый метод и заменить все continue с с return с. Это работало очень красиво, хотя было бы сложно, если бы в цикле были другие инструкции потока управления, такие как break и return.

---

Bill K: дразнить друг от друга итеративно; искать дублирование и устранять его. Воспользуйтесь полиморфными классами, чтобы заменить условное поведение. Используйте очень маленькие шаги. Да; это все хорошие советы с более широким применением, чем только этот конкретный случай.

---

Аарон: Либо использовать keepGoing флаг заменить continue или выбросить исключение. Я не пробовал это, но я думаю, что вариант Exception - очень хорошая альтернатива, и я не рассматривал.