Refactoring

Refactoring

Doppelter Code

In der Klasse SadMinecraftCommandArgumentParser wird in der Methode extractStateType ein String zu einem int konvertiert. Dasselbe wurde in der Methode extractBlockIdentifier mehrfach getan und daher bereits in eine Methode parseInt ausgelagert. extractStateType verwendet diese nicht und hat in der Dopplung dieses Codes auch noch einen Fehler eingebaut, indem die falsche Exception abgefangen wird um auf nicht konvertierbare Strings zu prüfen.

Zusammenführen des Codes

extractStateType wurde so umgeschrieben, dass es die Methode parseInt verwendet. Da das verwenden eines OptionalInt in extractStateType als sinnvoller als ein Optional<Integer> erachtet wurde, wurde zusätzlich die Methode parseInt und deren Aufrufe in extractBlockIdentifier entsprechend geändert.~~~~

Zu lange Methode (und doppelter Code)

In der Klasse SadMinecraftCommandArgumentParser ist die Methode extractBlockIdentifier mit einem 40 Zeilen langen Body (ohne Leerzeilen) relativ lang und unübersichtlich.

Struktur vereinfachen

Vereinfacht zusammengefasst setzt sich die Methode aus den folgenden Schritten zusammen:

• Parameter lesen und erkennen, ob der Parameter die X-Koordinate oder die Welt spezifiziert

  • wenn X: Setzen von X, Erkennen, ob die Welt implizit festgelegt wurde

  • wenn Welt: Setzen der Welt

• Wenn noch kein X gesetzt ist: Parameter lesen und X setzen

• Parameter lesen und Y setzen

• Parameter lesen und Z setzen

Zunächst lässt sich erkennen, dass die Struktur zu Beginn nicht sinnvoll strukturiert ist. Die ersten beiden Schritte ließen sich auch ausdrücken als:

• ist Parameter A oder B?

  • wenn A: setze A, lese B anders

  • wenn B: setze B

• wenn A nicht gesetzt: lese A anders

So wird offensichtlich, dass die beiden Fälle sehr ähnlich sind, aber anders behandelt werden. Sinnvoller wäre:

• ist Parameter A oder B?

  • wenn A: setze A, lese B anders

  • wenn B: setze B, lese A anders

Der Code wurde entsprechend geändert. Das hat jedoch nur 2 Zeilen eingespart.

Doppelten Code zusammenführen

Die meisten Code Dopplungen (in der ganzen Klasse) finden sich beim Lesen von Parametern und beim Prüfen auf Vorhandensein weiterer Parameter.

Das Lesen erfolgt stets durch Objects.requireNonNull(arguments.next()), das Prüfen durch arguments.hasNext() diese Aufrufe sollen in zwei Methoden ausgelagert werden. Damit der Aufruf des Attributs arguments danach nur noch von diesen beiden Methoden aus möglich ist, wird eine Superklasse extrahiert, die das Attribut als privat und die beiden Methoden als protected enthält. Der angepasste Code findet sich hier.

Das Rückgeben von leeren Optionals verhindert das Extrahieren von Methoden an diesen Stellen, ohne danach stattdessen den Rückgabetyp der neuen Methode zu prüfen und eventuell zurückzugeben. Der Code würde durch diese Auslagerung nicht schlanker. Daher wurde in Betracht gezogen, Exceptions zu verwenden, da sich die Codeausführung beim Werfen dieser gleich mehrere Ebenen nach oben bewegen kann. Da das Interface jedoch hierdurch nicht geändert werden soll, müssen die Exceptions noch in den Methoden, welche das Interface implementieren, gefangen und stattdessen ein leerer Optional zurückgegeben werden. Um dabei keine neue Code-Duplikation zu erzeugen, wird dieser Prozess in eine weitere Methode ausgelagert, die Code zur Erzeugung des Rückgabewerts und Ersatzcode zur Erzeugung des Ersatzrückgabewerts erwartet. Letzteres ist nötig, da nicht alle öffentlichen Methoden denselben Rückgabetyp besitzen. Die Unterscheidung dazwischen geschieht mit Generics. Ein Aufruf von throw returnDefault(); ermöglicht nun innerhalb solcher übergebenen Codesegmente praktisch ein Zurückgeben des Standardwertes aus beliebiger Aufruftiefe. Zum "Übergeben von Code", der einen Rückgabetyp berechnet, wird ein Supplier mit dem entsprechenden Typ implementiert. Die Änderungen finden sich hier.

Extrahieren von Methoden

Somit können nun auch mehrere Methoden von extractBlockIdentifier extrahiert werden. Eine neue innere Klasse von SadMinecraftCommandArgumentParser namens BlockIdentifierBuilder wurde erstellt, die als Attribute die Bestandteile des Ergebnisses enthält und durch die extrahierten Methoden "gefüllt" wird. Die Aufteilung in Methoden entspricht größtenteils der vorher erkannten Struktur:

• Parameter lesen und erkennen, ob der Parameter die X-Koordinate oder die Welt spezifiziert (getAndParseWorldIdentifierAndX)

  • wenn X: Setzen von X (parseX), Erkennen, ob die Welt implizit festgelegt wurde (parseWorldIdentifierImplicitlyByLocating)

  • wenn Welt: Setzen der Welt (parseWorldIdentifierExplicitly) und Lesen und Setzen von X (getAndParseX)

• Parameter lesen und Y setzen (getAndParseY)

• Parameter lesen und Z setzen (getAndParseZ)

Hier sind die Änderungen.