Methode extrahieren mit fortfahren

Question

Wir refaktorieren eine lange Methode; Es enthält eine lange for Schleife mit vielen continue Aussagen. Ich möchte nur die Extract-Methode Refactoring verwenden, aber Eclipse automatisierte ist nicht wissen, wie die bedingte Verzweigung zu behandeln. Ich auch nicht.

Unsere aktuelle Strategie ist es, eine keepGoing Flag (eine Instanzvariable, da wir zu Extrakt Methode wollen gehen) einzuführen, setzen Sie es an der Spitze der Schleife falsch, und ersetzen Sie alle weiterhin mit der Flagge Einstellung zu wahr, dann alle folgenden Sachen (auf verschiedenen Verschachtelungsebenen) in eine if (keepGoing) Klausel einwickeln. Führen Sie dann die verschiedenen Extraktionen durch, ersetzen Sie dann die keepGoing Zuordnungen mit frühen Rückgaben aus den extrahierten Methoden, und entfernen Sie dann die Markierung.

Gibt es einen besseren Weg?

aktualisieren: Als Reaktion auf die Kommentare - Ich kann den Code nicht teilen, aber hier ist ein anonymisierter Auszug:

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

Dies ist einer jener Spaß, wo kein einziges Muster Sie dorthin bringen wird.

Ich würde es iterativ arbeiten.

Zuerst würde ich versuchen, um zu sehen, ob ich eine frühe nicht weiter verwenden könnte, um eine dieser Ebenen von IFS zu entfernen. Es ist viel klarerer Code, um nach einer Bedingung zu suchen und früh zurückzukehren (oder in Ihrem Fall fortzusetzen), als tief verschachtelte Wenns zu haben.

Weiter Ich denke, ich würde einige der inneren Brocken nehmen und sehen, ob sie nicht in eine separate Methode extrahiert werden konnten. Es sieht so aus, als wären die ersten beiden großen Blöcke (innerhalb der "if (test2 (a, c)) {" und ihrer else-Anweisung) sehr ähnlich. Es gibt Cut-and-Paste-Logik, die identisch sein sollte.

Schließlich, nachdem das Zeug aufgeklärt ist, können Sie beginnen, Ihr tatsächliches Problem zu betrachten - Sie brauchen mehr Klassen. Diese gesamte Aussage ist wahrscheinlich eine dreizeilige polymorphe Methode in 3-5 Geschwisterklassen.

Es ist sehr nah an Wegwerfen und Neuschreiben von Code, sobald Sie Ihre tatsächlichen Klassen identifizieren, wird diese gesamte Methode verschwinden und durch etwas ersetzt werden, so einfach es tut weh. Allein die Tatsache, dass es sich um eine statische Hilfsmethode handelt, sollte Ihnen etwas sagen - Sie wollen keines von denen in dieser Art von Code.

Edit (Nach ein wenig mehr): Es gibt so viel hier würde es wirklich Spaß machen, durch zu gehen. Denken Sie daran, dass Sie, wenn Sie fertig sind, keine Code-Duplikation wollen - und ich bin mir ziemlich sicher, dass diese ganze Sache ohne eine einzige geschrieben werden könnte, wenn - ich denke, all Ihre ifs sind Fälle, die leicht durch Polymorphie behandelt werden können.

Oh, und als Antwort auf Ihre Frage der Eclipse nicht wollen - tun Sie nicht einmal automatisches Refactoring mit diesem Versuch, tun Sie es einfach von Hand. Das Zeug in diesem ersten if() muss in eine Methode herausgezogen werden, da es praktisch identisch mit der Klausel in else() ist!

Wenn ich so etwas mache, erstelle ich normalerweise eine neue Methode, verschiebe den Code von der if in die neue Methode (lasse nur einen Aufruf der neuen Methode im if), führe dann einen Test durch und vergewissere dich hat nichts kaputt gemacht.

dann gehen Sie Zeile für Zeile und überprüfen, um sicherzustellen, gibt es keinen Unterschied zwischen dem if und seinem else-Code. Wenn dies der Fall ist, kompensieren Sie es, indem Sie die Differenz als neue Variable an die Methode übergeben. Nachdem Sie sicher sind, dass alles identisch ist, ersetzen Sie die else-Klausel durch einen Aufruf. Nochmal testen. Wahrscheinlichkeiten sind an diesem Punkt ein paar zusätzliche Optimierungen werden offensichtlich, Sie werden wahrscheinlich verlieren die gesamte wenn durch die Kombination ihrer Logik mit der Variablen, die Sie übergeben, um die beiden Anrufe zu unterscheiden.

Machen Sie einfach so Sachen und iterieren. Der Trick beim Refactoring besteht darin, sehr kleine Schritte zu verwenden und zwischen den einzelnen Schritten zu testen, um sicherzustellen, dass sich nichts ändert.

Answer 2

continue ist im Grunde ein Analogon einer frühen Rückkehr, nicht wahr?

Jetzt muss ich zugeben, dass ich Ihrer aktuellen Strategie nicht ganz gefolgt bin, also hätte ich vielleicht einfach das wiederholt, was Sie gesagt haben. Wenn ja, dann ist meine Antwort, dass ich mir keinen besseren Weg vorstellen kann.

Answer 3

Wenn ich mit Ihrer Situation konfrontiert würde, würde ich mit anderen Refactoring-Techniken wie "ersetzen bedingt mit Polymorphismus" aussehen. Das heißt, sollten Sie eine Sache zu einer Zeit immer tun, wenn Sie also erste Methode extrahieren möchten Sie zwei Möglichkeiten:

1. Fügen Sie den „keepGoing“ Flag
2. werfen eine Ausnahme von der Methode

Von diesen beiden Optionen halte ich die keepGoing-Flagge für besser. Ich würde nicht aufhören zu refactoring, nachdem Sie die Methode extrahiert haben. Ich bin sicher, sobald Sie eine kleinere Methode haben, werden Sie eine Möglichkeit finden, diese Flagge zu entfernen und sauberere Logik zu haben.

Answer 4

Ich werde die Antworten hier zusammenfassen, während ich Bill Ks Antwort als die vollständigste akzeptiere. Aber jeder hatte etwas Gutes zu bieten, und ich könnte eines dieser Ansätze nutzen, wenn ich das nächste Mal mit einer solchen Situation konfrontiert werde.

---

mmyers: Der Schleifenkörper herausgeschnitten, in ein neues Verfahren kleben und mit return s all continue s ersetzen. Dies funktionierte sehr gut, obwohl es Probleme hätte, wenn es andere Kontrollflussanweisungen wie Pause und Rückkehr innerhalb der Schleife gäbe.

---

Bill K: es Necken auseinander iterativ; Suche nach Duplikaten und beseitige sie. Nutzen Sie polymorphe Klassen, um das bedingte Verhalten zu ersetzen. Verwenden Sie sehr kleine Schritte. Ja; Das ist alles gute Rat, mit breiterer Anwendbarkeit als nur in diesem speziellen Fall.

---

Aaron: Entweder die keepGoing Flag verwenden, um die continue zu ersetzen oder eine Ausnahme werfen. Ich habe das nicht versucht, aber ich denke, dass die Option "Exception" eine sehr nette Alternative ist, und eine, die ich nicht berücksichtigt hatte.