Refactoring in kleinen Schritten

Code in:JavaC#PythonJavaScript

Kurz gesagt: Refactoring ändert die Struktur von Code, ohne sein Verhalten zu ändern, in kleinen benannten Schritten wie Rename und Extract Method, mit grünen Tests nach jedem.

Problem

Die Rechnung, die niemand anfassen will

Der Online-Shop druckt Rechnungen mit der Methode Invoices.inv. Sie läuft seit zwei Jahren:

static String inv(String n, String street, String city, String zip, List<Item> items, String type) {
    double t = 0;
    for (Item i : items) {
        t = t + i.price() * i.qty();
    }
    double d = 0;
    if (type.equals("gold")) {
        d = t * 0.05;
    } else if (type.equals("company")) {
        if (t >= 500) {
            d = t * 0.10;
        }
    }
    String s = "Rechnung: " + n + "\n";
    s = s + street + ", " + zip + " " + city + "\n";
    s = s + String.format("Summe: %.2f €\n", t);
    s = s + String.format("Rabatt: %.2f €\n", d);
    s = s + String.format("Gesamt inkl. MwSt.: %.2f €", afterDiscount(t, d) * 1.20);
    return s;
}

static double afterDiscount(double t, double d) {
    return t - d;
}

Was ist t? Und d? Sechs Parameter hintereinander, vier davon Text: Wer Stadt und Postleitzahl vertauscht, bekommt eine Rechnung mit „Hamburg 20095“ an der falschen Stelle, und der Compiler sagt nichts. Die Kette von if über die Kundenart wächst weiter: Nächsten Monat kommen Partner mit 3 %. Und afterDiscount macht eine einzige Subtraktion, zwingt dich aber, woanders hinzuspringen, um sie zu sehen.

Du willst aufräumen. Aber die Rechnungen gehen an die Buchhaltung, und ein Fehler darin ist ein Fehler in den Steuern.

Die Idee

Refactoring: Die Form ändert sich, das Verhalten nicht

Refactoring ist eine Änderung an der Form des Codes, bei der das Verhalten gleich bleibt: Dieselben Eingaben ergeben dieselben Ausgaben. Es ist kein „Neuschreiben“. Es ist auch kein „ich habe nebenbei noch einen Fehler behoben“. Das sind andere Änderungen, die getrennt gemacht werden.

Es geschieht in kleinen benannten Schritten. Jeder Schritt ist so klein, dass er kaum schiefgehen kann, und nach jedem führst du die Tests aus (das Sicherheitsnetz aus der vorherigen Lektion). Die Namen stammen aus dem Katalog von Martin Fowler und sind in jeder Sprache und jeder IDE gleich: Rename, Extract Method, Inline, Introduce Parameter Object, Replace Conditional with Polymorphism.

Zuerst kommt das Netz. Vier Tests halten die vier Arten von Rechnung fest, die der Shop heute druckt: ein normaler Kunde, ein Goldkunde, eine Firma über und unter 500 €.

class InvoiceTest {
    List<Item> coffee = List.of(new Item("Kaffeebohnen", 12.50, 2), new Item("Tasse", 8.00, 1));
    List<Item> moreCoffee = List.of(new Item("Kaffeebohnen", 12.50, 4));
    List<Item> flour = List.of(new Item("Mehl 25 kg", 30.00, 20));

    @Test
    void regular() {
        assertEquals("Rechnung: Maria Becker\nLindenstraße 5, 20095 Hamburg\nSumme: 33.00 €\nRabatt: 0.00 €\nGesamt inkl. MwSt.: 39.60 €",
                Invoices.inv("Maria Becker", "Lindenstraße 5", "Hamburg", "20095", coffee, "regular"));
    }

    @Test
    void gold() {
        assertEquals("Rechnung: Jonas Keller\nBahnhofstraße 12, 80331 München\nSumme: 50.00 €\nRabatt: 2.50 €\nGesamt inkl. MwSt.: 57.00 €",
                Invoices.inv("Jonas Keller", "Bahnhofstraße 12", "München", "80331", moreCoffee, "gold"));
    }

    @Test
    void companyFrom500() {
        assertEquals("Rechnung: Bäckerei Morgenrot GmbH\nHauptstraße 80, 04109 Leipzig\nSumme: 600.00 €\nRabatt: 60.00 €\nGesamt inkl. MwSt.: 648.00 €",
                Invoices.inv("Bäckerei Morgenrot GmbH", "Hauptstraße 80", "Leipzig", "04109", flour, "company"));
    }

    @Test
    void companyUnder500() {
        assertEquals("Rechnung: Bäckerei Morgenrot GmbH\nHauptstraße 80, 04109 Leipzig\nSumme: 33.00 €\nRabatt: 0.00 €\nGesamt inkl. MwSt.: 39.60 €",
                Invoices.inv("Bäckerei Morgenrot GmbH", "Hauptstraße 80", "Leipzig", "04109", coffee, "company"));
    }
}

Ergebnis

OK    companyFrom500()
OK    companyUnder500()
OK    gold()
OK    regular()
Tests: 4, bestanden: 4, fehlgeschlagen: 0

Sieh es dir an

Schritt 1: Rename

Ein-Buchstaben-Namen werden zu Wörtern: t wird subtotal, d wird discount, s wird text, inv wird invoice. zip wird zu postcode, und type wird zu customerType.

static String invoice(String name, String street, String city, String postcode, List<Item> items, String customerType) {
    double subtotal = 0;
    for (Item item : items) {
        subtotal = subtotal + item.price() * item.qty();
    }
    double discount = 0;
    if (customerType.equals("gold")) {
        discount = subtotal * 0.05;
    } else if (customerType.equals("company")) {
        if (subtotal >= 500) {
            discount = subtotal * 0.10;
        }
    }
    String text = "Rechnung: " + name + "\n";
    text = text + street + ", " + postcode + " " + city + "\n";
    text = text + String.format("Summe: %.2f €\n", subtotal);
    text = text + String.format("Rabatt: %.2f €\n", discount);
    text = text + String.format("Gesamt inkl. MwSt.: %.2f €", afterDiscount(subtotal, discount) * 1.20);
    return text;
}

Die Tests

OK    companyFrom500()
OK    companyUnder500()
OK    gold()
OK    regular()
Tests: 4, bestanden: 4, fehlgeschlagen: 0

Rename ändert auch jede Stelle, die die Methode aufruft, die Tests eingeschlossen. Die erwarteten Rechnungen werden nicht angefasst.

In IntelliJ IDEA ist es Umschalt+F6: Die IDE findet jede Verwendung anhand der Typen, nicht anhand des Textes, und ändert nur diese.

Sieh es dir an

Schritt 2: Extract Method

Die Methode macht zwei getrennte Aufgaben, die mit dem Rechnungstext nichts zu tun haben: die Summe der Positionen und den Rabatt. Jede wandert in eine eigene Methode mit einem Namen, der sagt, was sie zurückgibt. Das ist Extract Method:

static String invoice(String name, String street, String city, String postcode, List<Item> items, String customerType) {
    double subtotal = itemsTotal(items);
    double discount = discountFor(customerType, subtotal);
    String text = "Rechnung: " + name + "\n";
    text = text + street + ", " + postcode + " " + city + "\n";
    text = text + String.format("Summe: %.2f €\n", subtotal);
    text = text + String.format("Rabatt: %.2f €\n", discount);
    text = text + String.format("Gesamt inkl. MwSt.: %.2f €", afterDiscount(subtotal, discount) * 1.20);
    return text;
}

static double itemsTotal(List<Item> items) {
    double sum = 0;
    for (Item item : items) {
        sum = sum + item.price() * item.qty();
    }
    return sum;
}

static double discountFor(String customerType, double subtotal) {
    double discount = 0;
    if (customerType.equals("gold")) {
        discount = subtotal * 0.05;
    } else if (customerType.equals("company")) {
        if (subtotal >= 500) {
            discount = subtotal * 0.10;
        }
    }
    return discount;
}

Die Tests

OK    companyFrom500()
OK    companyUnder500()
OK    gold()
OK    regular()
Tests: 4, bestanden: 4, fehlgeschlagen: 0

Jetzt liest sich invoice von oben nach unten wie eine Liste: wie viel es ist, wie hoch der Rabatt ist, wie der Text lautet. Die Einzelheiten sind einen Sprung entfernt, nur wenn du sie brauchst.

In IntelliJ IDEA: Zeilen markieren und Strg+Alt+M drücken (⌘⌥M auf dem Mac).

Die IDE schaut, welche Variablen in die markierten Zeilen hineingehen und welche herauskommen, und bildet daraus die Parameter und den Rückgabewert. Wenn du den Schritt von Hand machst, geht genau hier etwas schief.

Probier es aus

Sage die Ausgabe voraus

Jemand macht Extract Method von Hand. Der Rabatt wandert in eine Methode, die ihn in ihrem eigenen Parameter speichert, statt ihn zurückzugeben:

static String invoice(String name, String street, String city, String postcode, List<Item> items, String customerType) {
    double subtotal = itemsTotal(items);
    double discount = 0;
    applyDiscount(customerType, subtotal, discount);
    String text = "Rechnung: " + name + "\n";
    text = text + street + ", " + postcode + " " + city + "\n";
    text = text + String.format("Summe: %.2f €\n", subtotal);
    text = text + String.format("Rabatt: %.2f €\n", discount);
    text = text + String.format("Gesamt inkl. MwSt.: %.2f €", afterDiscount(subtotal, discount) * 1.20);
    return text;
}

static void applyDiscount(String customerType, double subtotal, double discount) {
    if (customerType.equals("gold")) {
        discount = subtotal * 0.05;
    } else if (customerType.equals("company")) {
        if (subtotal >= 500) {
            discount = subtotal * 0.10;
        }
    }
}

Antwort

gold() und companyFrom500() schlagen fehl

Alle vier bestehen

Nur companyFrom500() schlägt fehl

Prüfen

Genauer, für Neugierige

Ergebnis

FEHLERcompanyFrom500()
FEHLERgold()
OK    companyUnder500()
OK    regular()
Tests: 4, bestanden: 2, fehlgeschlagen: 2

discount in applyDiscount ist eine Kopie des Wertes. Der neue Wert bleibt dort drin und verschwindet, wenn die Methode endet, und draußen ist discount weiterhin 0. Wo der Rabatt ohnehin 0 ist (ein normaler Kunde, eine Firma unter 500 €), laufen die Tests durch. Deshalb hat das Netz für jeden Zweig des if einen Test. Der extrahierte Code muss den Wert zurückgeben, so wie es die IDE macht.

Sieh es dir an

Schritt 3: Inline

Das Gegenteil von Extract Method. afterDiscount macht eine Subtraktion, und sein Name sagt nicht mehr als subtotal - discount. Es wird an einer einzigen Stelle benutzt. Der Rumpf wandert dorthin, wo der Aufruf steht, und die Methode verschwindet. Das ist Inline:

static double afterDiscount(double subtotal, double discount) {
    return subtotal - discount;
}

static String invoice(String name, String street, String city, String postcode, List<Item> items, String customerType) {
    double subtotal = itemsTotal(items);
    double discount = discountFor(customerType, subtotal);
    String text = "Rechnung: " + name + "\n";
    text = text + street + ", " + postcode + " " + city + "\n";
    text = text + String.format("Summe: %.2f €\n", subtotal);
    text = text + String.format("Rabatt: %.2f €\n", discount);
    text = text + String.format("Gesamt inkl. MwSt.: %.2f €", (subtotal - discount) * 1.20);
    return text;
}

Die Tests

OK    companyFrom500()
OK    companyUnder500()
OK    gold()
OK    regular()
Tests: 4, bestanden: 4, fehlgeschlagen: 0

Extract Method und Inline widersprechen sich nicht. Du extrahierst, wenn der Name etwas erklärt, was der Code nicht sagt (itemsTotal). Du setzt es zurück, wenn der Name nur ein Sprung ist (afterDiscount). In IntelliJ IDEA ist es Strg+Alt+N.

Sieh es dir an

Schritt 4: Introduce Parameter Object

Der Name, die Straße, die Stadt und die Postleitzahl reisen immer zusammen, und doch sind es vier Parameter hintereinander. Eine Vertauschung fällt nicht auf:

Invoices.invoice("Maria Becker", "Lindenstraße 5", "20095", "Hamburg", coffee, "regular")

Ausgabe

Rechnung: Maria Becker
Lindenstraße 5, Hamburg 20095
Summe: 33.00 €
Rabatt: 0.00 €
Gesamt inkl. MwSt.: 39.60 €

Die vier werden zu einem Objekt: dem Kunden. Das ist Introduce Parameter Object.

Ein record schreibt die Felder, den Konstruktor und die Zugriffsmethoden:

record Customer(String name, String street, String city, String postcode) {}

static String invoice(Customer customer, List<Item> items, String customerType) {
    double subtotal = itemsTotal(items);
    double discount = discountFor(customerType, subtotal);
    String text = "Rechnung: " + customer.name() + "\n";
    text = text + customer.street() + ", " + customer.postcode() + " " + customer.city() + "\n";
    text = text + String.format("Summe: %.2f €\n", subtotal);
    text = text + String.format("Rabatt: %.2f €\n", discount);
    text = text + String.format("Gesamt inkl. MwSt.: %.2f €", (subtotal - discount) * 1.20);
    return text;
}

Die Tests

OK    companyFrom500()
OK    companyUnder500()
OK    gold()
OK    regular()
Tests: 4, bestanden: 4, fehlgeschlagen: 0

Die Aufrufe in den Tests ändern sich mit der Form, und die erwarteten Rechnungen bleiben gleich. Hier ist ein Aufruf durch die Schritte:

Invoices.inv("Jonas Keller", "Bahnhofstraße 12", "München", "80331", moreCoffee, "gold")
Invoices.invoice("Jonas Keller", "Bahnhofstraße 12", "München", "80331", moreCoffee, "gold")
Invoices.invoice(new Customer("Jonas Keller", "Bahnhofstraße 12", "München", "80331"), moreCoffee, "gold")
Invoices.invoice(new Customer("Jonas Keller", "Bahnhofstraße 12", "München", "80331"), moreCoffee, new Gold())

In Java ist eine Vertauschung im Konstruktor des record weiterhin möglich, aber jetzt an einer einzigen Stelle, an der es eine Prüfung geben kann (eine Postleitzahl hat fünf Ziffern). Java hat keine benannten Argumente. IntelliJ IDEA hat „Introduce Parameter Object“ im Menü Refactor.

Sieh es dir an

Schritt 5: Replace Conditional with Polymorphism

Übrig bleibt die Kette von if über die Kundenart in discountFor. Jede neue Art ist hier ein neuer Zweig, und morgen auch an anderen Stellen (die E-Mail, der Bericht). Jede Art wird zu einer Klasse, die ein gemeinsames Interface implementiert und kennt ihren eigenen Rabatt:

interface CustomerType {
    double discount(double subtotal);
}

class Regular implements CustomerType {
    @Override
    public double discount(double subtotal) {
        return 0;
    }
}

class Gold implements CustomerType {
    @Override
    public double discount(double subtotal) {
        return subtotal * 0.05;
    }
}

class Company implements CustomerType {
    @Override
    public double discount(double subtotal) {
        if (subtotal >= 500) {
            return subtotal * 0.10;
        }
        return 0;
    }
}

static String invoice(Customer customer, List<Item> items, CustomerType customerType) {
    double subtotal = itemsTotal(items);
    double discount = customerType.discount(subtotal);
    String text = "Rechnung: " + customer.name() + "\n";
    text = text + customer.street() + ", " + customer.postcode() + " " + customer.city() + "\n";
    text = text + String.format("Summe: %.2f €\n", subtotal);
    text = text + String.format("Rabatt: %.2f €\n", discount);
    text = text + String.format("Gesamt inkl. MwSt.: %.2f €", (subtotal - discount) * 1.20);
    return text;
}

Die Tests

OK    companyFrom500()
OK    companyUnder500()
OK    gold()
OK    regular()
Tests: 4, bestanden: 4, fehlgeschlagen: 0

Das ist Strategy aus dem Modul über Entwurfsmuster: Die Regel ist ein eigenes Objekt, und die Rechnung fragt nur. Wenn sich die Kundenart ändert, während das Programm läuft (nach der zehnten Bestellung wird er zum Goldkunden), führt derselbe Schritt zu State. Der Text "gold" aus der Datenbank wird an einer Stelle zu einem Objekt, am Eingang, mit einer Tabelle wie in der Lektion über Strategy.

record Customer(String name, String street, String city, String postcode) {}

interface CustomerType {
    double discount(double subtotal);
}

static String invoice(Customer customer, List<Item> items, CustomerType customerType) {
    double subtotal = itemsTotal(items);
    double discount = customerType.discount(subtotal);
    String text = "Rechnung: " + customer.name() + "\n";
    text = text + customer.street() + ", " + customer.postcode() + " " + customer.city() + "\n";
    text = text + String.format("Summe: %.2f €\n", subtotal);
    text = text + String.format("Rabatt: %.2f €\n", discount);
    text = text + String.format("Gesamt inkl. MwSt.: %.2f €", (subtotal - discount) * 1.20);
    return text;
}

Schritt für Schritt, in Worten

1. Die Funktion vor dem ersten Schritt: 18 Zeilen in einem Block. Unten stehen die vier Tests und die Summen der vier Rechnungen.
2. Rename: t wird zu subtotal, d wird zu discount, s wird zu text. Die Tests sind grün, die Summen bleiben gleich.
3. Extract Method: Die Schleife wandert nach itemsTotal, die if-Kette nach discountFor. An ihrer Stelle bleibt je eine Zeile.
4. Inline: afterDiscount hat nur abgezogen. Sein Rumpf kehrt in die Zeile zurück, und die Funktion verschwindet.
5. Introduce Parameter Object: Name, Straße, Stadt und Postleitzahl werden zu einem Kunden.
6. Replace Conditional with Polymorphism: Das if verschwindet. Jede Kundenart ist eine Klasse hinter CustomerType, und die Funktion ruft customerType.discount(subtotal) auf.
7. Fünf Schritte. Die Form von invoice ist eine andere, aber die vier Tests und die vier Rechnungen sind dieselben wie am Anfang.

Das Programm, das die vier Rechnungen druckt, lief mit dem Code von vor dem ersten Schritt und mit dem Code nach dem fünften. Die Ausgabe ist dieselbe:

Ausgabe vorher und nachher

Rechnung: Maria Becker
Lindenstraße 5, 20095 Hamburg
Summe: 33.00 €
Rabatt: 0.00 €
Gesamt inkl. MwSt.: 39.60 €

Rechnung: Jonas Keller
Bahnhofstraße 12, 80331 München
Summe: 50.00 €
Rabatt: 2.50 €
Gesamt inkl. MwSt.: 57.00 €

Rechnung: Bäckerei Morgenrot GmbH
Hauptstraße 80, 04109 Leipzig
Summe: 600.00 €
Rabatt: 60.00 €
Gesamt inkl. MwSt.: 648.00 €

Rechnung: Bäckerei Morgenrot GmbH
Hauptstraße 80, 04109 Leipzig
Summe: 33.00 €
Rabatt: 0.00 €
Gesamt inkl. MwSt.: 39.60 €

Probier es aus

Finde den Fehler

Jemand schreibt die Klasse für eine Firma so:

class Company implements CustomerType {
    @Override
    public double discount(double subtotal) {
        return subtotal * 0.10;
    }
}

Eine Firma bestellt Kaffee für 33 €:

Invoices.invoice(new Customer("Bäckerei Morgenrot GmbH", "Hauptstraße 80", "Leipzig", "04109"), coffee, new Company())

Ausgabe

Rechnung: Bäckerei Morgenrot GmbH
Hauptstraße 80, 04109 Leipzig
Summe: 33.00 €
Rabatt: 3.30 €
Gesamt inkl. MwSt.: 35.64 €

In welcher Zeile steckt der Fehler?

Wähle die Zeile mit dem Fehler.

1class Company implements CustomerType {2 @Override3 public double discount(double subtotal) {4 return subtotal * 0.10;5 }6}

Prüfen

Genauer, für Neugierige

Die Tests

FEHLERcompanyUnder500()
OK    companyFrom500()
OK    gold()
OK    regular()
Tests: 4, bestanden: 3, fehlgeschlagen: 1

Die Idee

Wann du aufhörst

Refactoring hat kein Ende, wenn du ihm keins setzt. Du hörst auf, wenn der Code für die kommende Änderung klar genug ist. Die Aufgabe ist, Partner mit 3 % zu ergänzen? Schritt 5 macht daraus eine neue Klasse. Die Aufgabe ist eine andere? Vielleicht brauchst du Schritt 5 gar nicht.

Es lohnt sich nicht, wenn:

• niemand den Code anfasst oder liest. Hässlicher Code, der funktioniert und sich nicht ändert, ist nicht dringend;
• es zwei Varianten gibt und sie nicht wachsen. Ein if ist klarer als ein Interface und zwei Klassen;
• unter dem Code kein Netz liegt. Dann ist der erste Schritt das Netz, nicht das Refactoring.

Und eine Regel: nicht mischen. Du änderst entweder die Form oder das Verhalten, nie beides in einem Schritt. Kent Beck nennt das „zwei Hüte“: Mit dem einen räumst du um und die Tests bleiben unberührt, mit dem anderen ergänzt du und schreibst einen neuen Test. Du wechselst den Hut mit Absicht.

Übung

Partner mit 3 %

Löse die Aufgabe in deinem Editor.

Partner. Der Shop ergänzt die Kundenart „Partner“ mit 3 % Rabatt auf jede Bestellung. Starte mit dem Code nach Schritt 5. Schreibe zuerst einen Test für einen Partner mit der Bestellung über 33 €, dann die Klasse Partner. Die anderen Klassen und invoice werden nicht angefasst.

Ausgabe

Rechnung: Maria Becker
Lindenstraße 5, 20095 Hamburg
Summe: 33.00 €
Rabatt: 0.99 €
Gesamt inkl. MwSt.: 38.41 €

Weitere Aufgaben

• Extract Method für den Text. Extrahiere die Adresszeile in eine Methode addressLine(Customer). Führe nach dem Schritt die Tests aus.
• Die andere Reihenfolge. Mach die fünf Schritte in anderer Reihenfolge: zuerst Introduce Parameter Object, dann Rename. Bestehen die Tests nach jedem? Welcher Schritt war früher einfacher?
• Die Art aus der Datenbank. Die Kundenart kommt als Text herein ("gold"). Schreibe die Tabelle, die ihn in ein Objekt verwandelt, und entscheide, was mit einer unbekannten Art passiert. Der heutige Code gibt für eine unbekannte Art einen Rabatt von 0: Behalte das mit einem Test bei.

Test

Teste dich

1. Was davon ist Refactoring?

Der Rabatt für einen Goldkunden wird 7 %Die Rechnung zeigt die MwSt. jetzt in einer eigenen ZeileDie Schleife für die Zwischensumme wandert in eine eigene Methode mit demselben Ergebnis

2. Nach Introduce Parameter Object lassen sich die Tests nicht mehr kompilieren, weil sie die Methode mit sechs Parametern aufrufen. Was tust du?

Auch die erwarteten Rechnungen ändern, damit sie passenDie Tests löschen und am Ende neue schreibenNur die Aufrufe in den Tests ändern; die erwarteten Rechnungen bleiben, wie sie sind

3. Wann ist Inline besser als Extract Method?

Wenn die Methode lang istNie: Mehr Methoden sind immer besserWenn die kleine Methode genauso viel sagt wie ihr Rumpf und beim Lesen nur einen Sprung dazufügt

4. Die Kette von if über die Kundenart wird zu einem Interface und einer Klasse je Art. Wie heißt das im Modul über Entwurfsmuster?

SingletonStrategy (oder State, wenn sich die Art ändert, während das Programm läuft)Observer

5. Fünf Schritte, nach jedem Tests. Beim vierten schlägt ein Test fehl. Was heißt das?

Du musst von vorn anfangenDer Test ist falschDer vierte Schritt hat das Verhalten geändert, und die ersten drei sind in Ordnung

Prüfen

Lektion von vorn beginnen

Verwandte Lektionen

• Baut auf: Namen im Code · Funktionen: eine Aufgabe, wenige Parameter, keine versteckten Seiteneffekte · Code-Smells: lange Methode, große Klasse, Duplikate, Primitive statt Typen, lange if-Ketten · Tests als Sicherheitsnetz
• Geht weiter in: SOLID ohne Dogma · Übung: eine chaotische Klasse, Schritt für Schritt aufgeräumt
• Aufgaben zum Thema: Sauberer Code und Refactoring