LU07.L10 - Refactoring-Review
| Paar | Urteil | Was passiert |
|---|---|---|
| 1 | Verhaltensänderung | namen.sort() gibt None zurück und sortiert die übergebene Liste an Ort und Stelle. Der Aufrufer bekommt None statt einer Liste - und seine Originalliste ist verändert |
| 2 | Verhaltensänderung | Der Generator ist nach sum aufgebraucht. max bekommt eine leere Folge und wirft ValueError |
| 3 | Verhaltensänderung | Der mutable Standardwert wird einmal beim Definieren erzeugt. Der zweite Aufruf sieht die Werte des ersten |
| 4 | Echtes Refactoring | Gleiche Liste, gleiche Reihenfolge, gleiche Objekte |
| 5 | Kommt darauf an | Siehe unten |
| 6 | Echtes Refactoring mit Nebengewinn | Gleiches Ergebnis. any hört beim ersten Treffer auf und baut keine Zwischenliste |
Die Tests, die es aufgedeckt hätten
def test_paar1_original_bleibt(): namen = ["Zoe", "Ali"] assert sortierte_namen(namen) == ["Ali", "Zoe"] assert namen == ["Zoe", "Ali"] # scheitert nachher def test_paar2_zweimal_lesbar(): werte = quadrate(1000) assert sum(werte) > 0 assert max(werte) > 0 # scheitert nachher mit ValueError def test_paar3_frischer_start(): assert sammeln(1) == [1] assert sammeln(2) == [2] # scheitert nachher: [1, 2]
Alle drei Tests haben dieselbe Bauart: Sie prüfen nicht nur den Rückgabewert, sondern auch was daneben passiert - der Zustand der Eingabe, die zweite Verwendung, der zweite Aufruf. Genau dort verstecken sich unerwünschte Nebeneffekte.
Zu Paar 5
Die Index-Fassung ist ein echtes Refactoring nur unter zwei Bedingungen:
kunde_idexistiert immer. Die Vorher-Version lässt Bestellungen ohne passenden Kunden still weg, die Nachher-Version wirftKeyError.idist eindeutig. Bei doppelten IDs liefert die Vorher-Version mehrere Zeilen pro Bestellung, die Nachher-Version nur eine.
bestellungen = [{"kunde_id": 1, "betrag": 10}, {"kunde_id": 9, "betrag": 20}] kunden = [{"id": 1, "name": "A"}, {"id": 1, "name": "A2"}] # vorher -> [('A', 10), ('A2', 10)] Bestellung 9 fällt still weg # nachher -> KeyError: 9
Beide Fassungen sind vertretbar - aber es sind verschiedene Fachregeln. Wer den Umbau macht, muss entscheiden, welche gelten soll, und das dokumentieren:
def zuordnen(bestellungen, kunden): index = {k["id"]: k for k in kunden} # bewusst: Bestellungen ohne passenden Kunden werden übersprungen (wie bisher) return [(index[b["kunde_id"]]["name"], b["betrag"]) for b in bestellungen if b["kunde_id"] in index]
Das ist der Kern von D1A. Ein Refactoring wird nicht dadurch richtig, dass die Tests grün sind - sondern dadurch, dass Sie wissen, welche Fälle die Tests nicht abdecken. Fehlende Schlüssel, Duplikate, leere Eingaben und mehrfache Verwendung sind die vier Stellen, an denen es fast immer passiert.
© Kevin Maurizi