====== 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 =====

<code python>
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]
</code>

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_id'' existiert immer.** Die Vorher-Version lässt Bestellungen ohne passenden Kunden still weg, die Nachher-Version wirft ''KeyError''.
  - **''id'' ist eindeutig.** Bei doppelten IDs liefert die Vorher-Version mehrere Zeilen pro Bestellung, die Nachher-Version nur eine.

<code python>
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
</code>

Beide Fassungen sind vertretbar - aber es sind **verschiedene** Fachregeln. Wer den Umbau macht, muss entscheiden, welche gelten soll, und das dokumentieren:

<code python>
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]
</code>

<WRAP center round important 90%>
**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.
</WRAP>

----
{{tag>M323-LU07 M323-D1A}}
[[https://creativecommons.org/licenses/by-nc-sa/4.0/ch/|{{https://i.creativecommons.org/l/by-nc-sa/4.0/88x31.png}}]] (c) Kevin Maurizi