-
Notifications
You must be signed in to change notification settings - Fork 0
Refactoring
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.
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.
In der Klasse SadMinecraftCommandArgumentParser ist die Methode extractBlockIdentifier mit einem 40 Zeilen langen Body (ohne Leerzeilen) relativ lang und unübersichtlich.
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.
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 ein Extrahieren von Methoden an diesen Stellen, ohne danach stattdessen den Rückgabetyp der neuen Methode zu prüfen und eventuell zurückzugeben zu müssen. Der Code würde durch diese Auslagerung nicht schlanker. Daher wurde in Betracht gezogen, Exceptions zu verwenden, da sich die Codeausführung beim Werfen selbiger 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 als Parameter 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.
Somit können nun auch mehrere Methoden von extractBlockIdentifier extrahiert werden. Eine neue innere Klasse von SadMinecraftCommandArgumentParser namens BlockIdentifierParser 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.
Die Methode loadFromBundle in der Klasse Loader ist zu lang und zu kompliziert. Ebenso ist sie nicht geeignet für die geplante Erweiterung der Loader Klasse um die Funktionalität, Plugins aus JAR Dateien zu laden. Um diese Funktionalität zu bekommen, muss die Klasse refactored werden. Desweiteren hätte es ebenso doppelten Code bedeutet. Aus diesem Grund wurde das Refactoring der Methode und der Beginn der Erweiterung um die JAR Lade Funktionalität in einem Schritt durchgeführt. Folgende Refactor Schritte wurden durchgeführt.
Code Abschnitte wie das Erstellen der Plugin Objekte und das Hinzufügen zur Liste der geladenen Plugins sind für Jar Plugins und gebundelte Plugins gleich. In der Funktion loadFromBundle wurde vor dem Refactoring beides durchgeführt. Die Methode createPlugin wurde dadurch neu erstellt.
private void createPlugin(String className) {
try {
pluginList.add(pluginFactory.createPlugin(className));
} catch (Exception e) {
e.printStackTrace();
}
}Die Änderungen wurden in diesem Commit durchgeführt. Im erwähnten Commit wurden noch mehr Refactorings durchgeführt.
Durch die Erweiterung der Loader Klasse durch eine JAR Plugin Lade Funktionalität konnte durch den Namen der Methode loadPluginsFromBundle nicht mehr festgestellt werden, für welchen Einsatzzweck sie geplant war, da sie nun für beide Funktionsweisen eingesetzt werden konnte.
Der Name der Methode loadFromBundle in der Klasse Loader ist seit der Erweiterung nicht mehr zutreffend. Sie hatte eigentlich nur die Aufgabe, die Classfiles aus einem übergebenen File Objekt zu laden. Bei der Erweiterung ist aufgefallen, dass diese Methode diese Aufgabe für beide Einsatzzwecke durchführen kann, sowohl für Classfiles im Classpath als auch Classfiles in Path Files. Für diesen Einsatzzweck wurde die Methode umbenannt. Um dem neuen Namen gerecht zu werden lautet ihr neuer Name nun loadPluginsFromDirectory.
Die Änderungen bei diesen Refactorings wurden im gleichen Commit durchgeführt.