Übung: eine chaotische Klasse, Schritt für Schritt aufgeräumt

Code in:JavaC#PythonJavaScript

Kurz gesagt: Eine Refactoring-Übung: Eine Scooter-Verleih-Klasse wird in fünf kleinen Schritten aufgeräumt (Namen, magische Zahlen, Geld, Abfrage getrennt vom Befehl, Tarife), während die Tests grün bleiben.

Problem

Die Scooter, die keiner anzufassen wagt

Ein Unternehmen, das E-Scooter vermietet, rechnet jede Fahrt mit der Klasse RideMgr ab. Sie läuft seit drei Jahren. Die Methode calc berechnet den Preis nach dem Tarif des Kunden (Pay-as-you-go, Student oder Monatskarte), lässt ab der vierten Fahrt des Tages die Entsperrgebühr weg, schlägt einen Nachtzuschlag auf, addiert den Umsatz und schickt einen Kassenbon:

class RideMgr {
    private final Map<String, Integer> c = new HashMap<>();
    private double s = 0;
    private final List<String> out = new ArrayList<>();

    // Der Preis: 1 € zum Entsperren und 0,15 € pro Minute.
    // Nachts steigt der Preis um 50 %.
    public double calc(String to, String t, int m, boolean f) {
        c.put(to, c.getOrDefault(to, 0) + 1);
        double p = 0;
        if (t.equals("payg")) {
            p = 1.0 + m * 0.2;
        } else if (t.equals("student")) {
            p = (1.0 + m * 0.2) * 0.8;
        } else if (t.equals("monthly")) {
            if (m > 45) {
                p = (m - 45) * 0.2;
            }
        }
        if (c.get(to) > 3 && !t.equals("monthly")) {
            p = p - 1.0;
        }
        if (f) {
            p = p + 0.5;
        }
        s = s + p;
        out.add(to + ": " + m + " Min., " + String.format("%.2f", p) + " €");
        return p;
    }

    public List<String> sent() {
        return out;
    }

    public double day() {
        return s;
    }
}

Nächsten Monat kommen zwei Änderungen:

• ein Firmentarif: keine Entsperrgebühr, 0,18 € pro Minute, kein Rabatt für die vierte Fahrt;
• der Preis vor dem Ende: Die App zeigt, was die Fahrt kosten wird, während sie noch läuft.

Die zweite Änderung geht mit dieser Klasse nicht. Das passiert, wenn die App calc aufruft, um den Preis zu zeigen, und dann noch einmal, um das Geld abzubuchen:

RideMgr rides = new RideMgr();
double shown = rides.calc("ana@example.com", "payg", 10, false);
System.out.println("Preis vor dem Ende: " + String.format("%.2f €", shown));
double charged = rides.calc("ana@example.com", "payg", 10, false);
System.out.println("Abgebucht: " + String.format("%.2f €", charged));
System.out.println("Kassenbons: " + rides.sent().size());
System.out.println("Umsatz: " + String.format("%.2f €", rides.day()));

Ausgabe

Preis vor dem Ende: 3.00 €
Abgebucht: 3.00 €
Kassenbons: 2
Umsatz: 6.00 €

Eine Fahrt, zwei Kassenbons und 6 € Umsatz. calc klingt nach einer Frage, aber es zählt, addiert und verschickt.

In dieser Lektion gibt es keine neue Idee. Es ist eine Übung: Die Klasse geht in kleinen Schritten durch die Werkzeuge des Moduls, und nach jedem Schritt lässt du die Tests laufen.

Sieh es dir an

Schritt 0: das Sicherheitsnetz

Zuerst das Netz, wie in der Lektion über Tests. Neun Tests halten fest, was die Klasse heute tut: den Preis für jeden Tarif, die Nacht, die vierte Fahrt, den Kassenbon und den Umsatz.

import java.util.List;
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertEquals;

class RideMgrTest {
    static final String EMAIL = "ana@example.com";

    RideMgr rides = new RideMgr();

    String ride(String plan, int minutes, boolean night) {
        return money(rides.calc(EMAIL, plan, minutes, night));
    }

    String money(double amount) {
        return String.format("%.2f", amount);
    }

    @Test
    void payAsYouGoRide() {
        assertEquals("3.00", ride("payg", 10, false));
    }

    @Test
    void studentRide() {
        assertEquals("2.40", ride("student", 10, false));
    }

    @Test
    void monthlyWithinFreeMinutes() {
        assertEquals("0.00", ride("monthly", 30, false));
    }

    @Test
    void monthlyOverFreeMinutes() {
        assertEquals("3.00", ride("monthly", 60, false));
    }

    @Test
    void nightSurcharge() {
        assertEquals("3.50", ride("payg", 10, true));
    }

    @Test
    void fourthRideUnlockIsFree() {
        ride("payg", 10, false);
        ride("payg", 10, false);
        ride("payg", 10, false);
        assertEquals("2.00", ride("payg", 10, false));
    }

    @Test
    void studentFourthRide() {
        // So ist es heute: der Rabatt ist der ganze 1 €, nicht 0,80 €.
        ride("student", 10, false);
        ride("student", 10, false);
        ride("student", 10, false);
        assertEquals("1.40", ride("student", 10, false));
    }

    @Test
    void receipt() {
        ride("payg", 10, false);
        assertEquals(List.of("ana@example.com: 10 Min., 3.00 €"), rides.sent());
    }

    @Test
    void revenue() {
        ride("payg", 10, false);
        ride("student", 10, false);
        assertEquals("5.40", money(rides.day()));
    }
}

Ergebnis

OK    fourthRideUnlockIsFree()
OK    monthlyOverFreeMinutes()
OK    monthlyWithinFreeMinutes()
OK    nightSurcharge()
OK    payAsYouGoRide()
OK    receipt()
OK    revenue()
OK    studentFourthRide()
OK    studentRide()
Tests: 9, bestanden: 9, fehlgeschlagen: 0

Die Tests rufen die Klasse nur über die Hilfsmethode ride auf. Wenn sich die Form der Klasse in den nächsten Schritten ändert, ändern sich die Hilfsmethode und die Aufrufe, aber nie die erwarteten Werte.

Einer der Tests ist merkwürdig. Ein Student zahlt 80 % des Preises, sein Entsperren kostet also 0,80 €. Bei der vierten Fahrt zieht die Klasse aber den vollen 1 € ab, und der Student zahlt 1,40 € statt 1,60 €. Ist das ein Fehler oder ein Geschenk? Das entscheidest nicht du, und nicht jetzt. Der Test hält fest, was heute passiert, mit einem Kommentar, und die Frage geht an den Product Owner. Beim Aufräumen ändert sich das Verhalten nicht, auch wenn es falsch aussieht.

Genauer, für Neugierige

Die Beträge werden als Text mit zwei Nachkommastellen verglichen ("2.40"), so wie der Kunde sie sieht. Ein Vergleich von Zahlen würde über die kleinen binären Fehler von double stolpern:

System.out.println((1.0 + 10 * 0.2) * 0.8);

Ausgabe

2.4000000000000004

Und in Schritt 3 ändert sich der Typ der Beträge. Der Text mit zwei Nachkommastellen ist davor und danach derselbe.

Sieh es dir an

Schritt 1: die Namen und das Flag

t, m, f, p, c und s werden zu Wörtern (Namen). Der wichtigste Name ist der der Methode: Sie berechnet nicht, sie beendet die Fahrt. Der ehrliche Name ist finishRide, und jetzt steht der Seiteneffekt im Namen. Die Klasse heißt jetzt RideBilling.

class RideBilling {
    private final Map<String, Integer> ridesToday = new HashMap<>();
    private double revenue = 0;
    private final List<String> receipts = new ArrayList<>();

    // Der Preis: 1 € zum Entsperren und 0,15 € pro Minute.
    // Nachts steigt der Preis um 50 %.
    public double finishRide(String email, String plan, int minutes, boolean night) {
        ridesToday.put(email, ridesToday.getOrDefault(email, 0) + 1);
        double price = 0;
        if (plan.equals("payg")) {
            price = 1.0 + minutes * 0.2;
        } else if (plan.equals("student")) {
            price = (1.0 + minutes * 0.2) * 0.8;
        } else if (plan.equals("monthly")) {
            if (minutes > 45) {
                price = (minutes - 45) * 0.2;
            }
        }
        if (ridesToday.get(email) > 3 && !plan.equals("monthly")) {
            price = price - 1.0;
        }
        if (night) {
            price = price + 0.5;
        }
        revenue = revenue + price;
        receipts.add(email + ": " + minutes + " Min., " + String.format("%.2f", price) + " €");
        return price;
    }

    public List<String> receipts() {
        return receipts;
    }

    public double revenue() {
        return revenue;
    }
}

Das Flag bleibt vorerst (Funktionen). Ein true im Aufruf sagt nichts. Nacht sind Daten über die Fahrt, nicht die Wahl einer anderen Aufgabe, deshalb werden daraus keine zwei Funktionen. Der Aufruf muss sagen, was er meint:

Java hat keine benannten Argumente. In den Tests sind die Werte Konstanten mit Namen, DAY und NIGHT, und die Hilfsmethode gibt sie weiter.

Vorher

String ride(String plan, int minutes, boolean night) {
    return money(rides.calc(EMAIL, plan, minutes, night));
}

@Test
void nightSurcharge() {
    assertEquals("3.50", ride("payg", 10, true));
}

Nachher

static final boolean DAY = false;
static final boolean NIGHT = true;

String ride(String plan, int minutes, boolean night) {
    return money(billing.finishRide(EMAIL, plan, minutes, night));
}

@Test
void nightSurcharge() {
    assertEquals("3.50", ride("payg", 10, NIGHT));
}

Die Tests

OK    fourthRideUnlockIsFree()
OK    monthlyOverFreeMinutes()
OK    monthlyWithinFreeMinutes()
OK    nightSurcharge()
OK    payAsYouGoRide()
OK    receipt()
OK    revenue()
OK    studentFourthRide()
OK    studentRide()
Tests: 9, bestanden: 9, fehlgeschlagen: 0

In IntelliJ IDEA ist Umbenennen Shift+F6: Es ändert auch die Aufrufe in den Tests.

Sieh es dir an

Schritt 2: die Zahlen und der Kommentar, der lügt

1.0, 0.2, 0.8, 45, 3 und 0.5 werden zu Konstanten mit Namen (magische Zahl → Konstante).

Der Kommentar über der Methode sagt 0,15 € pro Minute und 50 % Nachtzuschlag. Der Code berechnet 0,20 € und schlägt 0,50 € auf. Was stimmt? Die Tests halten fest, was der Code tut, und die Kunden zahlen seit Jahren danach. Der Kommentar lügt (Kommentare) und wird gelöscht. Die Konstanten sagen dasselbe und können nicht von allein veralten.

class RideBilling {
    private static final double UNLOCK_FEE = 1.00;
    private static final double PER_MINUTE = 0.20;
    private static final double STUDENT_SHARE = 0.80;
    private static final int MONTHLY_FREE_MINUTES = 45;
    private static final int PAID_UNLOCKS_PER_DAY = 3;
    private static final double NIGHT_SURCHARGE = 0.50;

    private final Map<String, Integer> ridesToday = new HashMap<>();
    private double revenue = 0;
    private final List<String> receipts = new ArrayList<>();

    public double finishRide(String email, String plan, int minutes, boolean night) {
        ridesToday.put(email, ridesToday.getOrDefault(email, 0) + 1);
        double price = 0;
        if (plan.equals("payg")) {
            price = UNLOCK_FEE + minutes * PER_MINUTE;
        } else if (plan.equals("student")) {
            price = (UNLOCK_FEE + minutes * PER_MINUTE) * STUDENT_SHARE;
        } else if (plan.equals("monthly")) {
            if (minutes > MONTHLY_FREE_MINUTES) {
                price = (minutes - MONTHLY_FREE_MINUTES) * PER_MINUTE;
            }
        }
        if (ridesToday.get(email) > PAID_UNLOCKS_PER_DAY && !plan.equals("monthly")) {
            price = price - UNLOCK_FEE;
        }
        if (night) {
            price = price + NIGHT_SURCHARGE;
        }
        revenue = revenue + price;
        receipts.add(email + ": " + minutes + " Min., " + String.format("%.2f", price) + " €");
        return price;
    }

    public List<String> receipts() {
        return receipts;
    }

    public double revenue() {
        return revenue;
    }
}

Die Tests

OK    fourthRideUnlockIsFree()
OK    monthlyOverFreeMinutes()
OK    monthlyWithinFreeMinutes()
OK    nightSurcharge()
OK    payAsYouGoRide()
OK    receipt()
OK    revenue()
OK    studentFourthRide()
OK    studentRide()
Tests: 9, bestanden: 9, fehlgeschlagen: 0

Die Tests bleiben unberührt: Weder der Name noch die Aufrufe haben sich geändert.

Probier es aus

Finde den Fehler

Ein Kollege macht denselben Schritt, vertraut aber dem Kommentar und behält ihn:

Wähle die Zeile mit dem Fehler.

1// Der Preis: 1 € zum Entsperren und 0,15 € pro Minute.2// Nachts steigt der Preis um 50 %.3private static final double UNLOCK_FEE = 1.00;4private static final double PER_MINUTE = 0.15;5private static final double STUDENT_SHARE = 0.80;6private static final int MONTHLY_FREE_MINUTES = 45;7private static final int PAID_UNLOCKS_PER_DAY = 3;8private static final double NIGHT_SURCHARGE = 0.50;

Prüfen

Genauer, für Neugierige

Die Tests

FEHLERfourthRideUnlockIsFree()
FEHLERmonthlyOverFreeMinutes()
FEHLERnightSurcharge()
FEHLERpayAsYouGoRide()
FEHLERreceipt()
FEHLERrevenue()
FEHLERstudentFourthRide()
FEHLERstudentRide()
OK    monthlyWithinFreeMinutes()
Tests: 9, bestanden: 1, fehlgeschlagen: 8

Sieh es dir an

Schritt 3: das Geld

Geld in einem double ist Primitive Obsession (Code-Smells). Heute ergibt es in keinem Test einen falschen Cent, weil jeder Betrag erst beim Ausgeben gerundet wird. Deshalb ist der Schritt ein Refactoring: Die Ausgabe ist dieselbe. Aber der Umsatz ist eine Summe aus Tausenden solcher Beträge, und dort häufen sich die kleinen Fehler.

double wird zu BigDecimal, aus einem String erzeugt. BigDecimal hat keine Operatoren: + und * werden zu add und multiply. Das wiederholte PER_MINUTE.multiply(BigDecimal.valueOf(minutes)) wird zu einer kleinen Methode perMinute.

class RideBilling {
    private static final BigDecimal UNLOCK_FEE = new BigDecimal("1.00");
    private static final BigDecimal PER_MINUTE = new BigDecimal("0.20");
    private static final BigDecimal STUDENT_SHARE = new BigDecimal("0.80");
    private static final int MONTHLY_FREE_MINUTES = 45;
    private static final int PAID_UNLOCKS_PER_DAY = 3;
    private static final BigDecimal NIGHT_SURCHARGE = new BigDecimal("0.50");

    private final Map<String, Integer> ridesToday = new HashMap<>();
    private BigDecimal revenue = BigDecimal.ZERO;
    private final List<String> receipts = new ArrayList<>();

    private static BigDecimal perMinute(int minutes) {
        return PER_MINUTE.multiply(BigDecimal.valueOf(minutes));
    }

    public BigDecimal finishRide(String email, String plan, int minutes, boolean night) {
        ridesToday.put(email, ridesToday.getOrDefault(email, 0) + 1);
        BigDecimal price = BigDecimal.ZERO;
        if (plan.equals("payg")) {
            price = UNLOCK_FEE.add(perMinute(minutes));
        } else if (plan.equals("student")) {
            price = UNLOCK_FEE.add(perMinute(minutes)).multiply(STUDENT_SHARE);
        } else if (plan.equals("monthly")) {
            if (minutes > MONTHLY_FREE_MINUTES) {
                price = perMinute(minutes - MONTHLY_FREE_MINUTES);
            }
        }
        if (ridesToday.get(email) > PAID_UNLOCKS_PER_DAY && !plan.equals("monthly")) {
            price = price.subtract(UNLOCK_FEE);
        }
        if (night) {
            price = price.add(NIGHT_SURCHARGE);
        }
        revenue = revenue.add(price);
        receipts.add(email + ": " + minutes + " Min., " + String.format("%.2f", price) + " €");
        return price;
    }

    public List<String> receipts() {
        return receipts;
    }

    public BigDecimal revenue() {
        return revenue;
    }
}

In den Tests ändert sich nur die Hilfsmethode, die den Betrag in Text verwandelt:

String money(BigDecimal amount) {
    return String.format("%.2f", amount);
}

Die Tests

OK    fourthRideUnlockIsFree()
OK    monthlyOverFreeMinutes()
OK    monthlyWithinFreeMinutes()
OK    nightSurcharge()
OK    payAsYouGoRide()
OK    receipt()
OK    revenue()
OK    studentFourthRide()
OK    studentRide()
Tests: 9, bestanden: 9, fehlgeschlagen: 0

Probier es aus

Ergänze

Der Preis pro Minute im Geldtyp der Sprache:

private static final BigDecimal PER_MINUTE = new BigDecimal();

Prüfen

Sieh es dir an

Schritt 4: die Abfrage getrennt vom Befehl

Jetzt die Änderung, die schon wartete: Der Preis vor dem Ende muss eine Abfrage sein, die nichts ändert (Befehl oder Abfrage). Die Berechnung wird in eine Methode price ausgelagert (Extract Method aus der Refactoring-Lektion). finishRide bleibt ein Befehl: Er fragt nach dem Preis, dann zählt er, addiert und verschickt.

Ein Kollege macht es so. Der Zähler wandert hinter den Preis, weil eine Fahrt erst zählt, wenn sie beendet ist:

Was sagen die neun Tests?

public BigDecimal price(String email, String plan, int minutes, boolean night) {
    BigDecimal price = BigDecimal.ZERO;
    if (plan.equals("payg")) {
        price = UNLOCK_FEE.add(perMinute(minutes));
    } else if (plan.equals("student")) {
        price = UNLOCK_FEE.add(perMinute(minutes)).multiply(STUDENT_SHARE);
    } else if (plan.equals("monthly")) {
        if (minutes > MONTHLY_FREE_MINUTES) {
            price = perMinute(minutes - MONTHLY_FREE_MINUTES);
        }
    }
    if (ridesToday.getOrDefault(email, 0) > PAID_UNLOCKS_PER_DAY && !plan.equals("monthly")) {
        price = price.subtract(UNLOCK_FEE);
    }
    if (night) {
        price = price.add(NIGHT_SURCHARGE);
    }
    return price;
}

public BigDecimal finishRide(String email, String plan, int minutes, boolean night) {
    BigDecimal price = price(email, plan, minutes, night);
    ridesToday.put(email, ridesToday.getOrDefault(email, 0) + 1);
    revenue = revenue.add(price);
    receipts.add(email + ": " + minutes + " Min., " + String.format("%.2f", price) + " €");
    return price;
}

Antwort

Alle neun bestehen: Der Zähler ist derselbe, er steht nur woanders

fourthRideUnlockIsFree() und studentFourthRide() schlagen fehl

Nur fourthRideUnlockIsFree() schlägt fehl

Prüfen

Genauer, für Neugierige

Die Tests

FEHLERfourthRideUnlockIsFree(): expected: <2.00> but was: <3.00>
FEHLERstudentFourthRide(): expected: <1.40> but was: <2.40>
OK    monthlyOverFreeMinutes()
OK    monthlyWithinFreeMinutes()
OK    nightSurcharge()
OK    payAsYouGoRide()
OK    receipt()
OK    revenue()
OK    studentRide()
Tests: 9, bestanden: 7, fehlgeschlagen: 2

Sieh es dir an

Schritt 4, zweiter Versuch

Das Rot zeigt auf den letzten Schritt. Du suchst den Fehler nicht in der ganzen Klasse: Du machst den Schritt rückgängig und machst ihn neu. Die Reihenfolge der Seiteneffekte war ein versteckter Teil des Verhaltens, und nur die Tests wussten davon.

price rechnet aus, die wievielte Fahrt des Tages das wird:

public BigDecimal price(String email, String plan, int minutes, boolean night) {
    int rideOfDay = ridesToday.getOrDefault(email, 0) + 1;
    BigDecimal price = BigDecimal.ZERO;
    if (plan.equals("payg")) {
        price = UNLOCK_FEE.add(perMinute(minutes));
    } else if (plan.equals("student")) {
        price = UNLOCK_FEE.add(perMinute(minutes)).multiply(STUDENT_SHARE);
    } else if (plan.equals("monthly")) {
        if (minutes > MONTHLY_FREE_MINUTES) {
            price = perMinute(minutes - MONTHLY_FREE_MINUTES);
        }
    }
    if (rideOfDay > PAID_UNLOCKS_PER_DAY && !plan.equals("monthly")) {
        price = price.subtract(UNLOCK_FEE);
    }
    if (night) {
        price = price.add(NIGHT_SURCHARGE);
    }
    return price;
}

public BigDecimal finishRide(String email, String plan, int minutes, boolean night) {
    BigDecimal price = price(email, plan, minutes, night);
    ridesToday.put(email, ridesToday.getOrDefault(email, 0) + 1);
    revenue = revenue.add(price);
    receipts.add(email + ": " + minutes + " Min., " + String.format("%.2f", price) + " €");
    return price;
}

Die Tests

OK    fourthRideUnlockIsFree()
OK    monthlyOverFreeMinutes()
OK    monthlyWithinFreeMinutes()
OK    nightSurcharge()
OK    payAsYouGoRide()
OK    receipt()
OK    revenue()
OK    studentFourthRide()
OK    studentRide()
Tests: 9, bestanden: 9, fehlgeschlagen: 0

Die Tests bleiben wieder unberührt: Sie rufen finishRide auf, und das tut dasselbe wie vorher.

Die ganze Klasse nach Schritt 4

class RideBilling {
    private static final BigDecimal UNLOCK_FEE = new BigDecimal("1.00");
    private static final BigDecimal PER_MINUTE = new BigDecimal("0.20");
    private static final BigDecimal STUDENT_SHARE = new BigDecimal("0.80");
    private static final int MONTHLY_FREE_MINUTES = 45;
    private static final int PAID_UNLOCKS_PER_DAY = 3;
    private static final BigDecimal NIGHT_SURCHARGE = new BigDecimal("0.50");

    private final Map<String, Integer> ridesToday = new HashMap<>();
    private BigDecimal revenue = BigDecimal.ZERO;
    private final List<String> receipts = new ArrayList<>();

    private static BigDecimal perMinute(int minutes) {
        return PER_MINUTE.multiply(BigDecimal.valueOf(minutes));
    }

    public BigDecimal price(String email, String plan, int minutes, boolean night) {
        int rideOfDay = ridesToday.getOrDefault(email, 0) + 1;
        BigDecimal price = BigDecimal.ZERO;
        if (plan.equals("payg")) {
            price = UNLOCK_FEE.add(perMinute(minutes));
        } else if (plan.equals("student")) {
            price = UNLOCK_FEE.add(perMinute(minutes)).multiply(STUDENT_SHARE);
        } else if (plan.equals("monthly")) {
            if (minutes > MONTHLY_FREE_MINUTES) {
                price = perMinute(minutes - MONTHLY_FREE_MINUTES);
            }
        }
        if (rideOfDay > PAID_UNLOCKS_PER_DAY && !plan.equals("monthly")) {
            price = price.subtract(UNLOCK_FEE);
        }
        if (night) {
            price = price.add(NIGHT_SURCHARGE);
        }
        return price;
    }

    public BigDecimal finishRide(String email, String plan, int minutes, boolean night) {
        BigDecimal price = price(email, plan, minutes, night);
        ridesToday.put(email, ridesToday.getOrDefault(email, 0) + 1);
        revenue = revenue.add(price);
        receipts.add(email + ": " + minutes + " Min., " + String.format("%.2f", price) + " €");
        return price;
    }

    public List<String> receipts() {
        return receipts;
    }

    public BigDecimal revenue() {
        return revenue;
    }
}

Sieh es dir an

Schritt 5: die Tarife

Übrig bleibt die Kette von if auf dem Text des Tarifs (lange if-Kette). Jeder neue Tarif ist ein neuer Zweig, und "monthly" wird an zwei Stellen geprüft. Jeder Tarif wird zu einer Klasse, die ihren Preis und ihren Rabatt für die vierte Fahrt kennt. Das ist Replace Conditional with Polymorphism aus der Refactoring-Lektion:

interface Plan {
    BigDecimal price(int minutes);

    BigDecimal loyaltyDiscount();
}

class PayAsYouGo implements Plan {
    @Override
    public BigDecimal price(int minutes) {
        return RideBilling.UNLOCK_FEE.add(RideBilling.perMinute(minutes));
    }

    @Override
    public BigDecimal loyaltyDiscount() {
        return RideBilling.UNLOCK_FEE;
    }
}

class Student implements Plan {
    private static final BigDecimal STUDENT_SHARE = new BigDecimal("0.80");

    @Override
    public BigDecimal price(int minutes) {
        return new PayAsYouGo().price(minutes).multiply(STUDENT_SHARE);
    }

    @Override
    public BigDecimal loyaltyDiscount() {
        // So ist es heute: der ganze 1 €, obwohl das Entsperren für Studierende 0,80 € kostet.
        return RideBilling.UNLOCK_FEE;
    }
}

class Monthly implements Plan {
    private static final int FREE_MINUTES = 45;

    @Override
    public BigDecimal price(int minutes) {
        return RideBilling.perMinute(Math.max(0, minutes - FREE_MINUTES));
    }

    @Override
    public BigDecimal loyaltyDiscount() {
        return BigDecimal.ZERO;
    }
}

Die Prüfung für die Monatskarte verschwindet: Sie liefert einfach einen Rabatt von 0. Das merkwürdige Verhalten beim Studenten steht jetzt in einer Zeile, mit einem Kommentar, der sagt, warum es so ist (der gute Kommentar aus der Lektion über Kommentare).

class RideBilling {
    static final BigDecimal UNLOCK_FEE = new BigDecimal("1.00");
    static final BigDecimal PER_MINUTE = new BigDecimal("0.20");
    private static final int PAID_UNLOCKS_PER_DAY = 3;
    private static final BigDecimal NIGHT_SURCHARGE = new BigDecimal("0.50");

    private final Map<String, Integer> ridesToday = new HashMap<>();
    private BigDecimal revenue = BigDecimal.ZERO;
    private final List<String> receipts = new ArrayList<>();

    static BigDecimal perMinute(int minutes) {
        return PER_MINUTE.multiply(BigDecimal.valueOf(minutes));
    }

    public BigDecimal price(String email, Plan plan, int minutes, boolean night) {
        int rideOfDay = ridesToday.getOrDefault(email, 0) + 1;
        BigDecimal price = plan.price(minutes);
        if (rideOfDay > PAID_UNLOCKS_PER_DAY) {
            price = price.subtract(plan.loyaltyDiscount());
        }
        if (night) {
            price = price.add(NIGHT_SURCHARGE);
        }
        return price;
    }

    public BigDecimal finishRide(String email, Plan plan, int minutes, boolean night) {
        BigDecimal price = price(email, plan, minutes, night);
        ridesToday.put(email, ridesToday.getOrDefault(email, 0) + 1);
        revenue = revenue.add(price);
        receipts.add(email + ": " + minutes + " Min., " + String.format("%.2f", price) + " €");
        return price;
    }

    public List<String> receipts() {
        return receipts;
    }

    public BigDecimal revenue() {
        return revenue;
    }
}

Der Anteil für Studenten und die Freiminuten wandern in den Tarif, der sie benutzt. Die Entsperrgebühr und der Preis pro Minute werden von mehreren Tarifen geteilt, deshalb bleiben sie in der Abrechnungsklasse und sind nicht mehr private.

In den Tests übergeben die Aufrufe ein Objekt statt Text:

assertEquals("3.50", ride("payg", 10, NIGHT));
assertEquals("3.50", ride(new PayAsYouGo(), 10, NIGHT));

Die Tests

OK    fourthRideUnlockIsFree()
OK    monthlyOverFreeMinutes()
OK    monthlyWithinFreeMinutes()
OK    nightSurcharge()
OK    payAsYouGoRide()
OK    receipt()
OK    revenue()
OK    studentFourthRide()
OK    studentRide()
Tests: 9, bestanden: 9, fehlgeschlagen: 0

Das ist Strategy aus dem Modul Entwurfsmuster: Die Regel ist ein eigenes Objekt, und die Abrechnung fragt nur.

Sieh es dir an

Die beiden Änderungen jetzt

Der Firmentarif ist eine neue Klasse. Die Abrechnungsklasse bleibt unberührt (offen für Erweiterung):

class Business implements Plan {
    private static final BigDecimal PER_MINUTE = new BigDecimal("0.18");

    @Override
    public BigDecimal price(int minutes) {
        return PER_MINUTE.multiply(BigDecimal.valueOf(minutes));
    }

    @Override
    public BigDecimal loyaltyDiscount() {
        return BigDecimal.ZERO;
    }
}

Der Preis vor dem Ende ist die Abfrage price. Wie oft du sie auch aufrufst, nichts wird gezählt, und kein Kassenbon geht raus:

public class Main {
    static final boolean DAY = false;
    static final boolean NIGHT = true;

    public static void main(String[] args) {
        RideBilling billing = new RideBilling();
        Plan payg = new PayAsYouGo();
        billing.finishRide("ana@example.com", payg, 10, DAY);
        billing.finishRide("ana@example.com", payg, 5, DAY);
        billing.finishRide("ana@example.com", payg, 12, NIGHT);
        System.out.println("Preis vor dem Ende: " + String.format("%.2f €", billing.price("ana@example.com", payg, 8, DAY)));
        System.out.println("Preis vor dem Ende: " + String.format("%.2f €", billing.price("ana@example.com", payg, 8, DAY)));
        System.out.println("Abgebucht: " + String.format("%.2f €", billing.finishRide("ana@example.com", payg, 8, DAY)));
        System.out.println("Firma, 12 Min.: " + String.format("%.2f €", billing.finishRide("office@example.com", new Business(), 12, DAY)));
        System.out.println("Kassenbons: " + billing.receipts().size());
    }
}

Ausgabe

Preis vor dem Ende: 1.60 €
Preis vor dem Ende: 1.60 €
Abgebucht: 1.60 €
Firma, 12 Min.: 2.16 €
Kassenbons: 5

Fünf Kassenbons: drei Fahrten vor der Vorschau, die vierte und die der Firma. Die zwei Vorschauen hinterlassen keine Spur, und sie zeigen genau, was danach abgebucht wird.

Die Idee

Vorher und nachher

Smell	Schritt	Lektion
nichts schützt das Verhalten	0: Tests, die das heutige Verhalten festhalten	das Sicherheitsnetz
Namen aus einem Buchstaben, calc, das nicht sagt, dass es verschickt	1: Rename	Namen
true im Aufruf	1: ein Argument, das sagt, was es meint	Funktionen
magische Zahlen	2: Konstanten mit Namen	Namen
ein Kommentar, der lügt	2: gelöscht	Kommentare
Geld in einem double	3: ein Typ für Geld	Code-Smells
eine Abfrage mit versteckten Seiteneffekten, eine lange Methode	4: Extract Method, Befehl und Abfrage	Funktionen, Refactoring
eine Kette von if auf dem Tarif als Text	5: Replace Conditional with Polymorphism	Code-Smells, Refactoring, SOLID

Die Klasse ist länger geworden, nicht kürzer. Aber jede Regel steht an einer Stelle und hat einen Namen, und jede Änderung vom Anfang ist eine Klasse.

Der Tag mit den Fahrten, gelaufen mit der Klasse vor Schritt 1 und mit der Klasse nach Schritt 5, gibt Zeile für Zeile dasselbe aus. Hier laufen beide Programme bei jeder Prüfung der Seite.

Vorher

RideMgr rides = new RideMgr();
rides.calc("ana@example.com", "payg", 10, false);
rides.calc("ivan@example.com", "student", 25, false);
rides.calc("ana@example.com", "payg", 5, false);
rides.calc("maria@example.com", "monthly", 60, true);
rides.calc("ana@example.com", "payg", 12, true);
rides.calc("ana@example.com", "payg", 8, false);

for (String receipt : rides.sent()) {
    System.out.println(receipt);
}
System.out.println("Umsatz: " + String.format("%.2f €", rides.day()));

Nachher

public class Main {
    static final boolean DAY = false;
    static final boolean NIGHT = true;

    public static void main(String[] args) {
        RideBilling billing = new RideBilling();
        Plan payg = new PayAsYouGo();
        billing.finishRide("ana@example.com", payg, 10, DAY);
        billing.finishRide("ivan@example.com", new Student(), 25, DAY);
        billing.finishRide("ana@example.com", payg, 5, DAY);
        billing.finishRide("maria@example.com", new Monthly(), 60, NIGHT);
        billing.finishRide("ana@example.com", payg, 12, NIGHT);
        billing.finishRide("ana@example.com", payg, 8, DAY);

        for (String receipt : billing.receipts()) {
            System.out.println(receipt);
        }
        System.out.println("Umsatz: " + String.format("%.2f €", billing.revenue()));
    }
}

Ausgabe

ana@example.com: 10 Min., 3.00 €
ivan@example.com: 25 Min., 4.80 €
ana@example.com: 5 Min., 2.00 €
maria@example.com: 60 Min., 3.50 €
ana@example.com: 12 Min., 3.90 €
ana@example.com: 8 Min., 1.60 €
Umsatz: 18.80 €

Die Idee

Wo du aufhörst

Du hörst auf, wenn die kommenden Änderungen leicht werden. Hier sind die beiden Änderungen eine Klasse und eine Zeile. Deshalb sind manche Schritte bewusst nicht gemacht:

• der Kassenbon in einer eigenen Klasse. Der Text des Kassenbons ist ein zweiter Grund, die Klasse zu ändern (ein Grund zur Änderung). Heute ändert er sich nicht. Sobald das Marketing ihn jeden Monat bearbeitet, wird er zu einer eigenen Klasse (Aufgabe 1);
• das Versenden hinter einem Interface. Die Kassenbons sind eine Liste, keine echte E-Mail. Wenn die echte E-Mail kommt, darf der Test sie nicht verschicken, und dann kommt die Abstraktion (von einer Abstraktion abhängen);
• das Verhalten beim Studenten. Das ist eine Änderung des Verhaltens, kein Aufräumen. Sie wird getrennt gemacht, mit einem eigenen Test, wenn das Produkt entscheidet.

Und die Regel aus der Refactoring-Lektion: Du änderst entweder die Form oder das Verhalten, nie beides in einem Schritt.

Übung

Mach selbst weiter

Löse die Aufgaben in deinem Editor, ausgehend von der Klasse nach Schritt 5. Lass nach jedem Schritt die neun Tests laufen.

1. Der Kassenbon in einer eigenen Klasse. Verschiebe den Text des Kassenbons und die Liste der verschickten Kassenbons in eine Klasse ReceiptBook. Die Abrechnungsklasse sagt ihr nur „send“. Die Tests bleiben unberührt.

Eine mögliche Lösung

class ReceiptBook {
    private final List<String> sent = new ArrayList<>();

    public void send(String email, int minutes, BigDecimal price) {
        sent.add(email + ": " + minutes + " Min., " + String.format("%.2f", price) + " €");
    }

    public List<String> sent() {
        return sent;
    }
}

class RideBilling {
    static final BigDecimal UNLOCK_FEE = new BigDecimal("1.00");
    static final BigDecimal PER_MINUTE = new BigDecimal("0.20");
    private static final int PAID_UNLOCKS_PER_DAY = 3;
    private static final BigDecimal NIGHT_SURCHARGE = new BigDecimal("0.50");

    private final Map<String, Integer> ridesToday = new HashMap<>();
    private BigDecimal revenue = BigDecimal.ZERO;
    private final ReceiptBook receipts = new ReceiptBook();

    static BigDecimal perMinute(int minutes) {
        return PER_MINUTE.multiply(BigDecimal.valueOf(minutes));
    }

    public BigDecimal price(String email, Plan plan, int minutes, boolean night) {
        int rideOfDay = ridesToday.getOrDefault(email, 0) + 1;
        BigDecimal price = plan.price(minutes);
        if (rideOfDay > PAID_UNLOCKS_PER_DAY) {
            price = price.subtract(plan.loyaltyDiscount());
        }
        if (night) {
            price = price.add(NIGHT_SURCHARGE);
        }
        return price;
    }

    public BigDecimal finishRide(String email, Plan plan, int minutes, boolean night) {
        BigDecimal price = price(email, plan, minutes, night);
        ridesToday.put(email, ridesToday.getOrDefault(email, 0) + 1);
        revenue = revenue.add(price);
        receipts.send(email, minutes, price);
        return price;
    }

    public List<String> receipts() {
        return receipts.sent();
    }

    public BigDecimal revenue() {
        return revenue;
    }
}

Die Tests

OK    fourthRideUnlockIsFree()
OK    monthlyOverFreeMinutes()
OK    monthlyWithinFreeMinutes()
OK    nightSurcharge()
OK    payAsYouGoRide()
OK    receipt()
OK    revenue()
OK    studentFourthRide()
OK    studentRide()
Tests: 9, bestanden: 9, fehlgeschlagen: 0

Das Programm für den Tag aus der Karte „Vorher und nachher“ gibt mit dieser Klasse dieselbe Ausgabe aus.

2. Der Tarif aus Text. Der Tarif kommt als Text ("payg") aus der Datenbank. Schreib die Tabelle, die ihn in ein Objekt verwandelt. Was hat die alte Klasse mit einem unbekannten Tarif gemacht? Halte es mit einem Test auf der alten Klasse fest, bevor du entscheidest, was die neue tun soll.

3. Nacht aus der Uhrzeit. Die App weiß nicht mehr, wann Nacht ist; sie übergibt die Startzeit. Die Regel „von 22:00 bis 6:00 Uhr“ muss an einer Stelle stehen. Schreib zuerst die Tests für 21:59, 22:00, 5:59 und 6:00.

4. Der Student. Das Produkt hat entschieden: Bei der vierten Fahrt bekommt ein Student 0,80 €, nicht 1 €. Das ist eine Änderung des Verhaltens: Ändere zuerst den erwarteten Wert im Test und sieh ihn rot, dann ändere eine Zeile im Tarif.

Test

Teste dich

1. Der Test für einen Studenten bei der vierten Fahrt erwartet 1,40 €, und die „richtige“ Antwort wäre wohl 1,60 €. Was machst du, während du die Klasse aufräumst?

Den Test löschen, weil er einen Fehler festhältDen erwarteten Wert auf 1,60 ändern und den Code im selben Schritt anpassenDen Test so lassen, mit einem Kommentar, und den Product Owner fragen. Entscheidet er anders, ist das eine eigene Änderung mit eigenem Test

2. Im Kommentar steht 0,15 € pro Minute, und der Code berechnet 0,20 €. Wem vertraust du?

Dem Kommentar: Er ist die DokumentationDem, was neuer istDem Code: Die Tests halten fest, was er tut, und die Kunden zahlen seit Jahren danach

3. Warum rufen die Tests die Klasse über eine Hilfsmethode auf und nicht direkt?

Weil das Test-Framework es verlangtDas macht sie schnellerWenn sich die Form der Klasse ändert, ändert sich eine einzige Stelle, und die erwarteten Werte bleiben gleich

4. Der erste Versuch bei Schritt 4 hat zwei Tests rot gemacht. Was machst du?

Den Schritt rückgängig machen und neu machen: Das Rot zeigt auf ihn, denn davor war alles grünDen Fehler in der ganzen Klasse suchenDie erwarteten Werte in den beiden Tests ändern

5. Nach dem Aufräumen ist der Firmentarif eine neue Klasse, und die Abrechnungsklasse bleibt unberührt. Welches Prinzip ist das?

Von einer Abstraktion abhängenEin Grund zur ÄnderungOffen für Erweiterung, geschlossen für Änderung

Prüfen

Lektion von vorn beginnen

Verwandte Lektionen

• Baut auf: Code-Smells: lange Methode, große Klasse, Duplikate, Primitive statt Typen, lange if-Ketten · Tests als Sicherheitsnetz · Refactoring in kleinen Schritten · SOLID ohne Dogma
• Aufgaben zum Thema: Sauberer Code und Refactoring