Ü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:

public class RideMgr
{
    private readonly Dictionary<string, int> c = new();
    public double S { get; private set; }
    public List<string> Out { get; } = new();

    // 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, bool f)
    {
        c[to] = c.GetValueOrDefault(to) + 1;
        double p = 0;
        if (t == "payg")
        {
            p = 1.0 + m * 0.2;
        }
        else if (t == "student")
        {
            p = (1.0 + m * 0.2) * 0.8;
        }
        else if (t == "monthly")
        {
            if (m > 45)
            {
                p = (m - 45) * 0.2;
            }
        }
        if (c[to] > 3 && t != "monthly")
        {
            p = p - 1.0;
        }
        if (f)
        {
            p = p + 0.5;
        }
        S = S + p;
        Out.Add($"{to}: {m} Min., {p:F2} €");
        return p;
    }
}

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:

var rides = new RideMgr();
double shown = rides.Calc("ana@example.com", "payg", 10, false);
Console.WriteLine($"Preis vor dem Ende: {shown:F2} €");
double charged = rides.Calc("ana@example.com", "payg", 10, false);
Console.WriteLine($"Abgebucht: {charged:F2} €");
Console.WriteLine($"Kassenbons: {rides.Out.Count}");
Console.WriteLine($"Umsatz: {rides.S:F2} €");

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.

using Xunit;

public class RideMgrTest
{
    private const string Email = "ana@example.com";
    private readonly RideMgr rides = new();

    private string Ride(string plan, int minutes, bool night)
    {
        return Money(rides.Calc(Email, plan, minutes, night));
    }

    private static string Money(double amount)
    {
        return amount.ToString("F2");
    }

    [Fact]
    public void PayAsYouGoRide()
    {
        Assert.Equal("3.00", Ride("payg", 10, false));
    }

    [Fact]
    public void StudentRide()
    {
        Assert.Equal("2.40", Ride("student", 10, false));
    }

    [Fact]
    public void MonthlyWithinFreeMinutes()
    {
        Assert.Equal("0.00", Ride("monthly", 30, false));
    }

    [Fact]
    public void MonthlyOverFreeMinutes()
    {
        Assert.Equal("3.00", Ride("monthly", 60, false));
    }

    [Fact]
    public void NightSurcharge()
    {
        Assert.Equal("3.50", Ride("payg", 10, true));
    }

    [Fact]
    public void FourthRideUnlockIsFree()
    {
        Ride("payg", 10, false);
        Ride("payg", 10, false);
        Ride("payg", 10, false);
        Assert.Equal("2.00", Ride("payg", 10, false));
    }

    [Fact]
    public 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);
        Assert.Equal("1.40", Ride("student", 10, false));
    }

    [Fact]
    public void Receipt()
    {
        Ride("payg", 10, false);
        Assert.Equal(new[] { "ana@example.com: 10 Min., 3.00 €" }, rides.Out);
    }

    [Fact]
    public void Revenue()
    {
        Ride("payg", 10, false);
        Ride("student", 10, false);
        Assert.Equal("5.40", Money(rides.S));
    }
}

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:

Console.WriteLine((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.

public class RideBilling
{
    private readonly Dictionary<string, int> ridesToday = new();
    public double Revenue { get; private set; }
    public List<string> Receipts { get; } = new();

    // 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, bool night = false)
    {
        ridesToday[email] = ridesToday.GetValueOrDefault(email) + 1;
        double price = 0;
        if (plan == "payg")
        {
            price = 1.0 + minutes * 0.2;
        }
        else if (plan == "student")
        {
            price = (1.0 + minutes * 0.2) * 0.8;
        }
        else if (plan == "monthly")
        {
            if (minutes > 45)
            {
                price = (minutes - 45) * 0.2;
            }
        }
        if (ridesToday[email] > 3 && plan != "monthly")
        {
            price = price - 1.0;
        }
        if (night)
        {
            price = price + 0.5;
        }
        Revenue = Revenue + price;
        Receipts.Add($"{email}: {minutes} Min., {price:F2} €");
        return price;
    }
}

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:

C# hat Standardwerte und benannte Argumente: Tag hat kein Argument, und Nacht ist night: true.

Vorher

private string Ride(string plan, int minutes, bool night)
{
    return Money(rides.Calc(Email, plan, minutes, night));
}

[Fact]
public void NightSurcharge()
{
    Assert.Equal("3.50", Ride("payg", 10, true));
}

Nachher

private string Ride(string plan, int minutes, bool night = false)
{
    return Money(billing.FinishRide(Email, plan, minutes, night));
}

[Fact]
public void NightSurcharge()
{
    Assert.Equal("3.50", Ride("payg", 10, night: true));
}

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 Visual Studio ist Umbenennen Strg+R, Strg+R (oder F2), in Rider 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.

public class RideBilling
{
    private const double UnlockFee = 1.00;
    private const double PerMinute = 0.20;
    private const double StudentShare = 0.80;
    private const int MonthlyFreeMinutes = 45;
    private const int PaidUnlocksPerDay = 3;
    private const double NightSurcharge = 0.50;

    private readonly Dictionary<string, int> ridesToday = new();
    public double Revenue { get; private set; }
    public List<string> Receipts { get; } = new();

    public double FinishRide(string email, string plan, int minutes, bool night = false)
    {
        ridesToday[email] = ridesToday.GetValueOrDefault(email) + 1;
        double price = 0;
        if (plan == "payg")
        {
            price = UnlockFee + minutes * PerMinute;
        }
        else if (plan == "student")
        {
            price = (UnlockFee + minutes * PerMinute) * StudentShare;
        }
        else if (plan == "monthly")
        {
            if (minutes > MonthlyFreeMinutes)
            {
                price = (minutes - MonthlyFreeMinutes) * PerMinute;
            }
        }
        if (ridesToday[email] > PaidUnlocksPerDay && plan != "monthly")
        {
            price = price - UnlockFee;
        }
        if (night)
        {
            price = price + NightSurcharge;
        }
        Revenue = Revenue + price;
        Receipts.Add($"{email}: {minutes} Min., {price:F2} €");
        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 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 const double UnlockFee = 1.00;4private const double PerMinute = 0.15;5private const double StudentShare = 0.80;6private const int MonthlyFreeMinutes = 45;7private const int PaidUnlocksPerDay = 3;8private const double NightSurcharge = 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 decimal, und die Literale bekommen ein m. Die Formeln bleiben, wie sie sind, denn decimal hat dieselben Operatoren. Im Kurs war Geld in C# von Anfang an decimal, aber diese Klasse ist alt und wurde mit double geschrieben.

public class RideBilling
{
    private const decimal UnlockFee = 1.00m;
    private const decimal PerMinute = 0.20m;
    private const decimal StudentShare = 0.80m;
    private const int MonthlyFreeMinutes = 45;
    private const int PaidUnlocksPerDay = 3;
    private const decimal NightSurcharge = 0.50m;

    private readonly Dictionary<string, int> ridesToday = new();
    public decimal Revenue { get; private set; }
    public List<string> Receipts { get; } = new();

    public decimal FinishRide(string email, string plan, int minutes, bool night = false)
    {
        ridesToday[email] = ridesToday.GetValueOrDefault(email) + 1;
        decimal price = 0;
        if (plan == "payg")
        {
            price = UnlockFee + minutes * PerMinute;
        }
        else if (plan == "student")
        {
            price = (UnlockFee + minutes * PerMinute) * StudentShare;
        }
        else if (plan == "monthly")
        {
            if (minutes > MonthlyFreeMinutes)
            {
                price = (minutes - MonthlyFreeMinutes) * PerMinute;
            }
        }
        if (ridesToday[email] > PaidUnlocksPerDay && plan != "monthly")
        {
            price = price - UnlockFee;
        }
        if (night)
        {
            price = price + NightSurcharge;
        }
        Revenue = Revenue + price;
        Receipts.Add($"{email}: {minutes} Min., {price:F2} €");
        return price;
    }
}

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

private static string Money(decimal amount)
{
    return amount.ToString("F2");
}

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 const decimal PerMinute = ;

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 decimal Price(string email, string plan, int minutes, bool night = false)
{
    decimal price = 0;
    if (plan == "payg")
    {
        price = UnlockFee + minutes * PerMinute;
    }
    else if (plan == "student")
    {
        price = (UnlockFee + minutes * PerMinute) * StudentShare;
    }
    else if (plan == "monthly")
    {
        if (minutes > MonthlyFreeMinutes)
        {
            price = (minutes - MonthlyFreeMinutes) * PerMinute;
        }
    }
    if (ridesToday.GetValueOrDefault(email) > PaidUnlocksPerDay && plan != "monthly")
    {
        price = price - UnlockFee;
    }
    if (night)
    {
        price = price + NightSurcharge;
    }
    return price;
}

public decimal FinishRide(string email, string plan, int minutes, bool night = false)
{
    decimal price = Price(email, plan, minutes, night);
    ridesToday[email] = ridesToday.GetValueOrDefault(email) + 1;
    Revenue = Revenue + price;
    Receipts.Add($"{email}: {minutes} Min., {price:F2} €");
    return price;
}

Antwort

FourthRideUnlockIsFree und StudentFourthRide schlagen fehl

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

Nur FourthRideUnlockIsFree schlägt fehl

Prüfen

Genauer, für Neugierige

Die Tests

FEHLERFourthRideUnlockIsFree
      Assert.Equal() Failure: Strings differ
                 ↓ (pos 0)
      Expected: "2.00"
      Actual:   "3.00"
                 ↑ (pos 0)
FEHLERStudentFourthRide
      Assert.Equal() Failure: Strings differ
                 ↓ (pos 0)
      Expected: "1.40"
      Actual:   "2.40"
                 ↑ (pos 0)
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 decimal Price(string email, string plan, int minutes, bool night = false)
{
    int rideOfDay = ridesToday.GetValueOrDefault(email) + 1;
    decimal price = 0;
    if (plan == "payg")
    {
        price = UnlockFee + minutes * PerMinute;
    }
    else if (plan == "student")
    {
        price = (UnlockFee + minutes * PerMinute) * StudentShare;
    }
    else if (plan == "monthly")
    {
        if (minutes > MonthlyFreeMinutes)
        {
            price = (minutes - MonthlyFreeMinutes) * PerMinute;
        }
    }
    if (rideOfDay > PaidUnlocksPerDay && plan != "monthly")
    {
        price = price - UnlockFee;
    }
    if (night)
    {
        price = price + NightSurcharge;
    }
    return price;
}

public decimal FinishRide(string email, string plan, int minutes, bool night = false)
{
    decimal price = Price(email, plan, minutes, night);
    ridesToday[email] = ridesToday.GetValueOrDefault(email) + 1;
    Revenue = Revenue + price;
    Receipts.Add($"{email}: {minutes} Min., {price:F2} €");
    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

public class RideBilling
{
    private const decimal UnlockFee = 1.00m;
    private const decimal PerMinute = 0.20m;
    private const decimal StudentShare = 0.80m;
    private const int MonthlyFreeMinutes = 45;
    private const int PaidUnlocksPerDay = 3;
    private const decimal NightSurcharge = 0.50m;

    private readonly Dictionary<string, int> ridesToday = new();
    public decimal Revenue { get; private set; }
    public List<string> Receipts { get; } = new();

    public decimal Price(string email, string plan, int minutes, bool night = false)
    {
        int rideOfDay = ridesToday.GetValueOrDefault(email) + 1;
        decimal price = 0;
        if (plan == "payg")
        {
            price = UnlockFee + minutes * PerMinute;
        }
        else if (plan == "student")
        {
            price = (UnlockFee + minutes * PerMinute) * StudentShare;
        }
        else if (plan == "monthly")
        {
            if (minutes > MonthlyFreeMinutes)
            {
                price = (minutes - MonthlyFreeMinutes) * PerMinute;
            }
        }
        if (rideOfDay > PaidUnlocksPerDay && plan != "monthly")
        {
            price = price - UnlockFee;
        }
        if (night)
        {
            price = price + NightSurcharge;
        }
        return price;
    }

    public decimal FinishRide(string email, string plan, int minutes, bool night = false)
    {
        decimal price = Price(email, plan, minutes, night);
        ridesToday[email] = ridesToday.GetValueOrDefault(email) + 1;
        Revenue = Revenue + price;
        Receipts.Add($"{email}: {minutes} Min., {price:F2} €");
        return price;
    }
}

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:

public interface IPlan
{
    decimal Price(int minutes);

    decimal LoyaltyDiscount();
}

public class PayAsYouGo : IPlan
{
    public decimal Price(int minutes)
    {
        return RideBilling.UnlockFee + minutes * RideBilling.PerMinute;
    }

    public decimal LoyaltyDiscount()
    {
        return RideBilling.UnlockFee;
    }
}

public class Student : IPlan
{
    private const decimal StudentShare = 0.80m;

    public decimal Price(int minutes)
    {
        return new PayAsYouGo().Price(minutes) * StudentShare;
    }

    public decimal LoyaltyDiscount()
    {
        // So ist es heute: der ganze 1 €, obwohl das Entsperren für Studierende 0,80 € kostet.
        return RideBilling.UnlockFee;
    }
}

public class Monthly : IPlan
{
    private const int FreeMinutes = 45;

    public decimal Price(int minutes)
    {
        return Math.Max(0, minutes - FreeMinutes) * RideBilling.PerMinute;
    }

    public decimal LoyaltyDiscount()
    {
        return 0;
    }
}

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).

public class RideBilling
{
    public const decimal UnlockFee = 1.00m;
    public const decimal PerMinute = 0.20m;
    private const int PaidUnlocksPerDay = 3;
    private const decimal NightSurcharge = 0.50m;

    private readonly Dictionary<string, int> ridesToday = new();
    public decimal Revenue { get; private set; }
    public List<string> Receipts { get; } = new();

    public decimal Price(string email, IPlan plan, int minutes, bool night = false)
    {
        int rideOfDay = ridesToday.GetValueOrDefault(email) + 1;
        decimal price = plan.Price(minutes);
        if (rideOfDay > PaidUnlocksPerDay)
        {
            price = price - plan.LoyaltyDiscount();
        }
        if (night)
        {
            price = price + NightSurcharge;
        }
        return price;
    }

    public decimal FinishRide(string email, IPlan plan, int minutes, bool night = false)
    {
        decimal price = Price(email, plan, minutes, night);
        ridesToday[email] = ridesToday.GetValueOrDefault(email) + 1;
        Revenue = Revenue + price;
        Receipts.Add($"{email}: {minutes} Min., {price:F2} €");
        return price;
    }
}

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:

Assert.Equal("3.50", Ride("payg", 10, night: true));
Assert.Equal("3.50", Ride(new PayAsYouGo(), 10, night: true));

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):

public class Business : IPlan
{
    private const decimal PerMinute = 0.18m;

    public decimal Price(int minutes)
    {
        return minutes * PerMinute;
    }

    public decimal LoyaltyDiscount()
    {
        return 0;
    }
}

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

var billing = new RideBilling();
var payg = new PayAsYouGo();
billing.FinishRide("ana@example.com", payg, 10);
billing.FinishRide("ana@example.com", payg, 5);
billing.FinishRide("ana@example.com", payg, 12, night: true);
Console.WriteLine($"Preis vor dem Ende: {billing.Price("ana@example.com", payg, 8):F2} €");
Console.WriteLine($"Preis vor dem Ende: {billing.Price("ana@example.com", payg, 8):F2} €");
Console.WriteLine($"Abgebucht: {billing.FinishRide("ana@example.com", payg, 8):F2} €");
Console.WriteLine($"Firma, 12 Min.: {billing.FinishRide("office@example.com", new Business(), 12):F2} €");
Console.WriteLine($"Kassenbons: {billing.Receipts.Count}");

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

var 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);

foreach (string receipt in rides.Out)
{
    Console.WriteLine(receipt);
}
Console.WriteLine($"Umsatz: {rides.S:F2} €");

Nachher

var billing = new RideBilling();
var payg = new PayAsYouGo();
billing.FinishRide("ana@example.com", payg, 10);
billing.FinishRide("ivan@example.com", new Student(), 25);
billing.FinishRide("ana@example.com", payg, 5);
billing.FinishRide("maria@example.com", new Monthly(), 60, night: true);
billing.FinishRide("ana@example.com", payg, 12, night: true);
billing.FinishRide("ana@example.com", payg, 8);

foreach (string receipt in billing.Receipts)
{
    Console.WriteLine(receipt);
}
Console.WriteLine($"Umsatz: {billing.Revenue:F2} €");

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

public class ReceiptBook
{
    public List<string> Sent { get; } = new();

    public void Send(string email, int minutes, decimal price)
    {
        Sent.Add($"{email}: {minutes} Min., {price:F2} €");
    }
}

public class RideBilling
{
    public const decimal UnlockFee = 1.00m;
    public const decimal PerMinute = 0.20m;
    private const int PaidUnlocksPerDay = 3;
    private const decimal NightSurcharge = 0.50m;

    private readonly Dictionary<string, int> ridesToday = new();
    public decimal Revenue { get; private set; }
    private readonly ReceiptBook receipts = new();
    public List<string> Receipts => receipts.Sent;

    public decimal Price(string email, IPlan plan, int minutes, bool night = false)
    {
        int rideOfDay = ridesToday.GetValueOrDefault(email) + 1;
        decimal price = plan.Price(minutes);
        if (rideOfDay > PaidUnlocksPerDay)
        {
            price = price - plan.LoyaltyDiscount();
        }
        if (night)
        {
            price = price + NightSurcharge;
        }
        return price;
    }

    public decimal FinishRide(string email, IPlan plan, int minutes, bool night = false)
    {
        decimal price = Price(email, plan, minutes, night);
        ridesToday[email] = ridesToday.GetValueOrDefault(email) + 1;
        Revenue = Revenue + price;
        receipts.Send(email, minutes, 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

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