Zum Inhalt
wannacode

Refactoring in kleinen Schritten

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:

public static string Inv(string n, string street, string city, string zip, List<Item> items, string type)
{
decimal t = 0;
foreach (Item i in items)
{
t = t + i.Price * i.Qty;
}
decimal d = 0;
if (type == "gold")
{
d = t * 0.05m;
}
else if (type == "company")
{
if (t >= 500)
{
d = t * 0.10m;
}
}
string s = "Rechnung: " + n + "\n";
s = s + street + ", " + zip + " " + city + "\n";
s = s + $"Summe: {t:F2} €\n";
s = s + $"Rabatt: {d:F2} €\n";
s = s + $"Gesamt inkl. MwSt.: {AfterDiscount(t, d) * 1.20m:F2} €";
return s;
}
static decimal AfterDiscount(decimal t, decimal 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 €.

public class InvoiceTest
{
List<Item> coffee = [new("Kaffeebohnen", 12.50m, 2), new("Tasse", 8.00m, 1)];
List<Item> moreCoffee = [new("Kaffeebohnen", 12.50m, 4)];
List<Item> flour = [new("Mehl 25 kg", 30.00m, 20)];
[Fact]
public void Regular()
{
Assert.Equal("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"));
}
[Fact]
public void Gold()
{
Assert.Equal("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"));
}
[Fact]
public void CompanyFrom500()
{
Assert.Equal("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"));
}
[Fact]
public void CompanyUnder500()
{
Assert.Equal("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.

public static string Invoice(string name, string street, string city, string postcode, List<Item> items, string customerType)
{
decimal subtotal = 0;
foreach (Item item in items)
{
subtotal = subtotal + item.Price * item.Qty;
}
decimal discount = 0;
if (customerType == "gold")
{
discount = subtotal * 0.05m;
}
else if (customerType == "company")
{
if (subtotal >= 500)
{
discount = subtotal * 0.10m;
}
}
string text = "Rechnung: " + name + "\n";
text = text + street + ", " + postcode + " " + city + "\n";
text = text + $"Summe: {subtotal:F2} €\n";
text = text + $"Rabatt: {discount:F2} €\n";
text = text + $"Gesamt inkl. MwSt.: {AfterDiscount(subtotal, discount) * 1.20m:F2} €";
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 Visual Studio ist es Strg+R, Strg+R (oder F2), in Rider Umschalt+F6. Der Compiler weiß, welches Inv welches ist, also ändert sich genau dieses, nicht jedes Stück Text Inv.

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:

public static string Invoice(string name, string street, string city, string postcode, List<Item> items, string customerType)
{
decimal subtotal = ItemsTotal(items);
decimal discount = DiscountFor(customerType, subtotal);
string text = "Rechnung: " + name + "\n";
text = text + street + ", " + postcode + " " + city + "\n";
text = text + $"Summe: {subtotal:F2} €\n";
text = text + $"Rabatt: {discount:F2} €\n";
text = text + $"Gesamt inkl. MwSt.: {AfterDiscount(subtotal, discount) * 1.20m:F2} €";
return text;
}
static decimal ItemsTotal(List<Item> items)
{
decimal sum = 0;
foreach (Item item in items)
{
sum = sum + item.Price * item.Qty;
}
return sum;
}
static decimal DiscountFor(string customerType, decimal subtotal)
{
decimal discount = 0;
if (customerType == "gold")
{
discount = subtotal * 0.05m;
}
else if (customerType == "company")
{
if (subtotal >= 500)
{
discount = subtotal * 0.10m;
}
}
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 Visual Studio: Zeilen markieren und Strg+R, Strg+M drücken (oder Strg+. und „Extract method“). In Rider: Strg+Alt+M.

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:

public static string Invoice(string name, string street, string city, string postcode, List<Item> items, string customerType)
{
decimal subtotal = ItemsTotal(items);
decimal discount = 0;
ApplyDiscount(customerType, subtotal, discount);
string text = "Rechnung: " + name + "\n";
text = text + street + ", " + postcode + " " + city + "\n";
text = text + $"Summe: {subtotal:F2} €\n";
text = text + $"Rabatt: {discount:F2} €\n";
text = text + $"Gesamt inkl. MwSt.: {AfterDiscount(subtotal, discount) * 1.20m:F2} €";
return text;
}
static void ApplyDiscount(string customerType, decimal subtotal, decimal discount)
{
if (customerType == "gold")
{
discount = subtotal * 0.05m;
}
else if (customerType == "company")
{
if (subtotal >= 500)
{
discount = subtotal * 0.10m;
}
}
}
Antwort
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 decimal AfterDiscount(decimal subtotal, decimal discount)
{
return subtotal - discount;
}
public static string Invoice(string name, string street, string city, string postcode, List<Item> items, string customerType)
{
decimal subtotal = ItemsTotal(items);
decimal discount = DiscountFor(customerType, subtotal);
string text = "Rechnung: " + name + "\n";
text = text + street + ", " + postcode + " " + city + "\n";
text = text + $"Summe: {subtotal:F2} €\n";
text = text + $"Rabatt: {discount:F2} €\n";
text = text + $"Gesamt inkl. MwSt.: {(subtotal - discount) * 1.20m:F2} €";
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 Rider ist es Strg+Alt+N, in Visual Studio Strg+. und „Inline method“.

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 Properties und den Konstruktor:

public record Customer(string Name, string Street, string City, string Postcode);
public static string Invoice(Customer customer, List<Item> items, string customerType)
{
decimal subtotal = ItemsTotal(items);
decimal discount = DiscountFor(customerType, subtotal);
string text = "Rechnung: " + customer.Name + "\n";
text = text + customer.Street + ", " + customer.Postcode + " " + customer.City + "\n";
text = text + $"Summe: {subtotal:F2} €\n";
text = text + $"Rabatt: {discount:F2} €\n";
text = text + $"Gesamt inkl. MwSt.: {(subtotal - discount) * 1.20m:F2} €";
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 C# kann der Konstruktor auch mit Namen aufgerufen werden: new Customer(Name: "...", City: "...", ...). Dann spielt die Reihenfolge keine Rolle.

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:

public interface ICustomerType
{
decimal Discount(decimal subtotal);
}
public class Regular : ICustomerType
{
public decimal Discount(decimal subtotal)
{
return 0;
}
}
public class Gold : ICustomerType
{
public decimal Discount(decimal subtotal)
{
return subtotal * 0.05m;
}
}
public class Company : ICustomerType
{
public decimal Discount(decimal subtotal)
{
if (subtotal >= 500)
{
return subtotal * 0.10m;
}
return 0;
}
}
public static string Invoice(Customer customer, List<Item> items, ICustomerType customerType)
{
decimal subtotal = ItemsTotal(items);
decimal discount = customerType.Discount(subtotal);
string text = "Rechnung: " + customer.Name + "\n";
text = text + customer.Street + ", " + customer.Postcode + " " + customer.City + "\n";
text = text + $"Summe: {subtotal:F2} €\n";
text = text + $"Rabatt: {discount:F2} €\n";
text = text + $"Gesamt inkl. MwSt.: {(subtotal - discount) * 1.20m:F2} €";
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.

public record Customer(string Name, string Street, string City, string Postcode);
public interface ICustomerType
{
decimal Discount(decimal subtotal);
}
public static string Invoice(Customer customer, List<Item> items, ICustomerType customerType)
{
decimal subtotal = ItemsTotal(items);
decimal discount = customerType.Discount(subtotal);
string text = "Rechnung: " + customer.Name + "\n";
text = text + customer.Street + ", " + customer.Postcode + " " + customer.City + "\n";
text = text + $"Summe: {subtotal:F2} €\n";
text = text + $"Rabatt: {discount:F2} €\n";
text = text + $"Gesamt inkl. MwSt.: {(subtotal - discount) * 1.20m:F2} €";
return text;
}
Schritt für Schritt, in Worten
  1. Die Funktion vor dem ersten Schritt: 23 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 ICustomerType, 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:

public class Company : ICustomerType
{
public decimal Discount(decimal subtotal)
{
return subtotal * 0.10m;
}
}

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.

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?
2. Nach Introduce Parameter Object lassen sich die Tests nicht mehr kompilieren, weil sie die Methode mit sechs Parametern aufrufen. Was tust du?
3. Wann ist Inline besser als Extract Method?
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?
5. Fünf Schritte, nach jedem Tests. Beim vierten schlägt ein Test fehl. Was heißt das?

Erstellt von Ivo Gergov im Rahmen der Kampagne von Evo Tech für zugängliche Programmierausbildung ·