Método de extracto con continue

Question

Estamos refabricando un método largo; contiene un bucle largo for con muchas declaraciones continue. Me gustaría simplemente utilizar la refactorización Extract Method, pero la automatizada de Eclipse no sabe cómo manejar la bifurcación condicional. Yo tampoco.

Nuestra estrategia actual consiste en introducir un keepGoing bandera (una variable de instancia ya que vamos a querer método de extracto de), establecido en false en la parte superior del bucle, y reemplazar cada Continuar con el establecimiento del indicador es cierto, y luego ajusta todo lo siguiente (en diferentes niveles de anidación) dentro de una cláusula if (keepGoing). A continuación, realice las diversas extracciones, luego reemplace las asignaciones keepGoing con retornos anticipados de los métodos extraídos, luego deshágase de la bandera.

¿Hay una manera mejor?

actualización: En respuesta a los comentarios - que no puedo compartir el código, pero aquí es un extracto anónima:

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

Este es uno de esos divertidos donde ningún patrón individual te llevará hasta allí.

Me gustaría trabajar en ello de forma iterativa.

Primero, trataría de ver si no puedo usar uno de los primeros para continuar eliminando uno de esos niveles de ifs. Es un código mucho más claro verificar una condición y regresar temprano (o en su caso continuar) que tener un if anidado.

Luego, creo que tomaría algunos de los fragmentos internos y vería si no se pueden extraer en un método diferente. Parece que los primeros dos grandes bloques (dentro de "if (test2 (a, c)) {" y su declaración else) son muy similares. Existe una lógica de cortar y pegar que debería ser la misma.

Finalmente, después de que se aclaren esas cosas, puede comenzar a analizar su problema real: necesita más clases. Esta declaración completa es probablemente un método polimórfico de tres líneas en 3-5 clases de hermanos.

Está muy cerca de tirar y reescribir el código, una vez que identifiques tus clases reales, todo este método desaparecerá y será reemplazado por algo tan simple que duele. Solo el hecho de que es un método de utilidad estático debería estar diciéndote algo: no quieres uno de esos en este tipo de código.

Editar (Después de mirar un poco más): Hay tantas cosas aquí que sería realmente divertido de pasar. Recuerda que cuando hayas terminado no quieres duplicar el código, y estoy bastante seguro de que todo esto podría escribirse sin una sola, si creo que todos tus ifs son casos que podrían/​​deberían manejarse fácilmente por polimorfismo.

Ah, y como respuesta a su pregunta de eclipse que no quiere hacerlo, ni siquiera INTENTAR la refacturación automática con esta, simplemente hágalo a mano. Las cosas dentro de ese primer if() necesitan ser extraídas en un método porque es virtualmente idéntico a la cláusula en su else()!

Cuando hago algo como esto, generalmente creo un nuevo método, muevo el código del método if al nuevo (dejando solo una llamada al nuevo método dentro del if), luego ejecuto una prueba y me aseguro de que no rompió nada

, vaya línea por línea y verifique que no haya diferencia entre el código if y its else. Si lo hay, hay que compensarlo pasando la diferencia como una nueva variable al método. Después de estar seguro de que todo es idéntico, reemplace la cláusula else con una llamada. Prueba de nuevo. Lo más probable es que en este momento se vuelvan obvias algunas optimizaciones adicionales, lo más probable es que pierda todo al combinar su lógica con la variable que pasó para diferenciar las dos llamadas.

Solo sigue haciendo cosas así e iterando. El truco con la refactorización es utilizar Pasos muy pequeños y realizar pruebas entre cada paso para garantizar que no haya cambiado nada.

Answer 2

continue es, básicamente, un análogo de un pronto retorno, ¿verdad?

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

Ahora debo admitir que no seguí exactamente su estrategia actual, por lo que acabo de repetir lo que usted dijo. Si es así, entonces mi respuesta es que no puedo pensar en una mejor manera.

Answer 3

Si me enfrentara a su situación, consideraría utilizar otras técnicas de refactorización como "reemplazar condicional con polimorfismo". Dicho esto siempre se debe hacer una cosa a la vez, por lo que si primero desea extraer método tiene dos opciones:

1. agregar la opción "keepGoing"
2. lanzar una excepción a partir del método

De estas dos opciones, creo que la bandera de keepGoing es mejor. No dejaría de refactorizar después de extraer el método. Estoy seguro de que una vez que tenga un método más pequeño, encontrará una manera de eliminar esta bandera y tener una lógica más limpia.

Answer 4

Voy a resumir las respuestas aquí, al tiempo que acepto la respuesta de Bill K como la más completa. Pero todos tenían algo bueno que ofrecer, y podría usar cualquiera de estos enfoques la próxima vez que enfrente este tipo de situación.

---

mmyers: Cortar el cuerpo del bucle, pegarlo en un nuevo método y reemplazar todas las continue s con return s. Esto funcionó muy bien, aunque tendría problemas si hubiera otras sentencias de flujo de control, como break y return, dentro del ciclo.

---

Bill K: Tease lo distingue de forma iterativa; busca la duplicación y elimínalo. Aproveche las clases polimórficas para reemplazar el comportamiento condicional. Use muy pequeños pasos. Sí; esto es todo un buen consejo, con una aplicabilidad más amplia que solo este caso específico.

---

Aaron, puede coger la bandera keepGoing para reemplazar el continue o lanzar una excepción. No probé esto, pero creo que la opción Excepción es una muy buena alternativa, y una que no había considerado.