Einfache Refactorings – Teil 3

if-Anweisungen vereinfachen – Extract Method

Ein weiteres Anwendungsgebiet für das Extract Method Refactoring sind Bedingungen von Verzweigungen. Das folgende Listing zeigt einen Ausschnitt aus einer Weckeranwendung.

if (DateTime.Now >= weckZeit) {
    using (var soundPlayer = new SoundPlayer(@"c:\Windows\Media\chimes.wav")) {
        soundPlayer.Play();
    }
    timer.Stopp();
}

Die Bedingung der if-Anweisung erschließt sich beim Lesen spätestens, wenn wir den Inhalt der Verzweigung lesen. Da wird eine Sounddatei abgespielt und ein Timer gestoppt, so dass alles danach aussieht, als würde dieser Code ausgeführt, wenn die Weckzeit erreicht ist. Vergleichen Sie den ursprünglichen Code mit folgender Variante:

if (WeckzeitErreicht()) {
    using (var soundPlayer = new SoundPlayer(@"c:\Windows\Media\chimes.wav")) {
        soundPlayer.Play();
    }
    timer.Stopp();
}


private bool WeckzeitErreicht() {
    return DateTime.Now >= weckZeit;
}

Nun kann ich die Bedingung der if-Anweisung lesen und verstehe sofort den Sinn der Verzweigung. Auch hier ist wieder eine weitere Abstraktionsebene eingezogen, die es dem Leser ermöglicht, das Abstraktionsniveau beizubehalten oder in die Details abzusteigen. Der Leser hat die Wahl, was er vor dem Refactoring nicht hatte. Im Beispiel kommt es allerdings noch zu einer Verletzung des Prinzip Single Level of Abstraction. Die Bedingung der Verzweigung steht nun auf hohem Abstraktionsniveau, während der Rumpf der bedingten Ausführung sich auf niedrigem Abstraktionsniveau befindet. Ein weitere Extract Method passt die Niveaus an:

if (WeckzeitErreicht()) {
    Wecken();
}

Fazit

Benutzen Sie Extract Method, um die Bedingung einer if-Anweisung leichter verständlich zu machen. Das höhere Abstraktionsniveau macht die Bedingung für den Leser besser lesbar, vorausgesetzt, der Name ist gut gewählt. Darüberhinaus bietet sich durch Extract Method die Gelegenheit, eine DRY Verletzung zu beheben, sprich eine Dopplung kann eliminiert werden.

Eingaben sichtbar machen – Introduce Parameter

Um eine Methode verstehen zu können, müssen die Eingaben und Ausgaben der Methode bekannt sein. Der erste Blick geht in der Regel zur Signatur der Methode. Dort wird anhand des Rückgabetyps der Methode deutlich, ob die Methode ein Ergebnis liefert und von welchem Typ dieses ist. Über die Methodenparameter wird deutlich, welches die Eingaben der Methode sind. Aus den Eingabewerten entsteht durch die Methode der Ausgabewert – EVA – Eingabe, Verarbeitung, Ausgabe. Im folgenden Listing ist dies einfach ersichtlich:

public int Summe(int[] werte)

Aufgrund dieser Signatur erwarten wir von der Methode, dass sie die Werte aus dem Integerarray aufsummiert und als Ergebnis zurück liefert. Was ist Ihre Erwartung beim Anblick der folgenden Signatur?

public int Summe()

Aufgrund der Signatur alleine bleibt unklar, wie die Methode arbeitet. Mit den Kenntnissen in Objektorientierung können wir als Entwickler erahnen, dass diese Methode auf Zustand der Klasse arbeiten wird. Hier ist das gesamte Listing:

public class Calculator
{
    private readonly int[] _werte;

    public Calculator(int[] werte) {
        _werte = werte;
    }

    public int Summe() {
        var result = 0;
        foreach (var i in _werte) {
            result += i;
        }
        return result;
    }
}

Beim Lesen des vollständigen Quellcodes wird deutlich, dass die zu summierenden Werte über den Konstruktor in die Klasse reingereicht werden. Die Eingaben der Methode Summe bestehen also nicht nur aus den Parametern sondern auch aus den Feldern der Klasse. Manchmal müssen Methoden auf Zustand der Klasse arbeiten. Wo dies jedoch vermeidbar ist, sollten die Eingaben alle als Parameter übergeben werden, da die Methode auf diese Weise leichter verständlich ist. Es muss dann nämlich nicht die gesamte Klasse betrachtet werden, sondern ein isolierter Blick auf die Methode genügt. Der zu betrachtende Ausschnitt des Codes ist kleiner. Des Weiteren ist der Mechanismus einfacher. Ein Leser muss nicht den Konstruktor und das Feld kennen, bevor er versteht, wie die Methode arbeitet. Es genügt ein isolierter Blick auf die Methode.

Mit Hilfe des Introduce Parameter Refactorings kann die Methode so verändert werden, dass ihr die benötigten Werte als Parameter reingereicht werden. Dazu markieren Sie in der Methode das Feld und starten in Ihrer IDE das Introduce Parameter Refactoring.

public int Summe() {
    var result = 0;
    foreach (var i in _werte) {
        result += i;
    }
    return result;
}

Die IDE fügt einen Parameter vom Typ des Feldes, hier int[], in die Methodensignatur ein und versucht anschließend, bei allen Aufrufern der Methode Summe das Feld _werte als Parameter einzusetzen. An dieser Stelle sollte deutlich werden, dass dieses Refactoring häufig begrenzt ist auf den Sichtbarkeitsbereich einer Klasse. Innerhalb der Klasse kann die Ersetzung vorgenommen werden. Außerhalb der Klasse kann bei Aufrufern das Feld der Klasse in der Regel nur eingesetzt werden, wenn es public ist.

Ein weiteres Beispiel soll verdeutlichen, wie schwierig es ist, Methoden zu verstehen, die neben ihren Parametern auch auf Feldern der Klasse als Eingabe arbeiten:

public string[] CsvTabellieren(string[] csvZeilen) {
    _csvZeilen = csvZeilen;

    RecordsErstellen();
    int[] breiten = SpaltenbreitenErmitteln();
    TabelleErstellen(breiten);

    return _tabelle;
}

Die Methode CsvTabellieren erhält ein Array von Strings als Eingabe. Jeder String dieses Arrays ist eine CSV Zeile. Als Ergebnis liefert die Methode eine hübsch formatierte Tabelle dieser CSV Zeilen, wiederum als Array von Strings.

Die Funktionsweise der Lösung können wir aufgrund der aufgerufenen Methoden erahnen. Offensichtlich wird aus den CSV Zeilen eine Menge von Records erstellt. Anschließend werden die Breiten der einzelnen Spalten ermittelt, um dann zuletzt die Tabelle zu erstellen. Unklar ist, wie die Werte in die einzelnen Methoden übergeben werden. Die Methode RecordsErstellen hat weder Parameter noch gibt sie ein Ergebnis zurück (es sei denn, dieses wird ignoriert, was bei C# syntaktisch erlaubt ist). In der Zeile davor ist erkennbar, dass die Eingabe in einem Feld abgelegt wird. Dass es sich um ein Feld handelt, vermuten wir aufgrund der Namenskonvention: der Bezeichner beginnt mit einem Unterstrich und wird daher wohl ein Feld der Klasse sein.

Offensichtlich werden die Werte hier also über Felder der Klasse übergeben. Das Ergebnis von RecordsErstellen wird vermutlich ebenfalls über ein Feld der Klasse an SpaltenbreitenErmitteln übergeben. Bei der Methode TabelleErstellen werden dann beide Übergabemethoden, mittels Feld und mittels Parameter, noch vermischt. Das Beispiel ist konstruiert… Sie werden eine solche Vorgehensweise aber vermutlich trotzdem auch in Ihrer Codebasis finden. Durch Introduce Parameter Refactorings können die Methoden leicht so modifiziert werden, dass den Methoden alle benötigten Eingaben per Parameter übergeben werden. Das Ergebnis sind wie folgt aus:

public string[] CsvTabellieren(string[] csvZeilen) {
    _csvZeilen = csvZeilen;

    RecordsErstellen(_csvZeilen);
    int[] breiten = SpaltenbreitenErmitteln(_records);
    TabelleErstellen(breiten, _records);

    return _tabelle;
}

Nun erhalten die Methoden alle benötigten Eingaben als Parameter. Die Ausgaben von RecordsErstellen und TabelleErstellen werden jedoch noch über Felder weitergegeben. Hier hilft nun leider kein automatisiertes Refactoring, sondern die nachfolgende Änderung muss per Hand durchgeführt werden. Die Ausgaben der Methoden müssen mit return zurückgegeben werden, statt die Werte in Feldern abzulegen. Im Ergebnis sieht die Methode nach den Refactorings wie folgt aus:

public string[] CsvTabellieren(string[] csvZeilen) {
    var records = RecordsErstellen(csvZeilen);
    int[] breiten = SpaltenbreitenErmitteln(records);
    var tabelle = TabelleErstellen(breiten, records);

    return tabelle;
}

Fazit

Benutzen Sie Introduce Parameter, um möglichst alle Eingaben einer Methode in die Parameterliste aufzunehmen. Dadurch wird die Methode leichter verständlich.

Weiter zu Teil 4 der Serie.

 

 

8 Gedanken zu „Einfache Refactorings – Teil 3“

    • Ich sehe den Vorteil. Der Nachteil ist aber, dass es so schnell zu einer Methodenschlacht kommen kann (kleinste Expressions werden in eine Methode gewrapped). Methoden sind dann sinnvoll, wenn sie von mehr als einem Consumer verwendet werden oder zumindest das Potential haben, von mehr als einer Stelle verwendet zu werden (Code-Reusability). In diesem Beispiel wird die Methode aber hauptsächlich nur dazu verwendet, die Readability zu erhöhen. Das geht m.M.n. nach auch mittels anderer – weniger Code invasiver – Techniken, z.B. mit einem einfach Kommentar über der If-Anweisung

      Antworten
  1. Wieso sind Methoden nur sinnvoll, wenn sie mehr als einen Consumer oder das Potential dazu haben? Welchem Wert sollte das dienen? Wiederverwendbarkeit ist für mich kein Wert. Wenn sie sich ergibt, schön. Aber darauf ziele ich nicht ab, weil der Code dadurch eher zu generalisiert, zu kompliziert wird. KISS – Keep it simple stupid.

    Kommentare erfüllen nicht den selben Zweck wie Methoden, da sie keine Abstraktionsebene einziehen.

    Und: natürlich gilt es auch beim Extract Method mit Augenmaß vorzugehen, damit es nicht zur Methodenschlacht kommt. Lassen Sie uns das konkrete Beispiel oben diskutieren: ist die Methode dort zu viel des Guten? Ich meine nein. Kann man es übertreiben beim Extract Method? Natürlich.

    Antworten
  2. Hallo,

    Dadurch hatt die obere Methode eine Abhängigkeit von Weckzeiterreicht(im Sinne vom PoMo und IOSP).
    Man könnte das natürlich per delegaten übergeben, aber dadurch würde sich evtl. die Parameterliste aufblähen.

    Was wäre denn hier sinnvoller?

    Antworten
    • Das Ergebnis sieht bislang so aus:
      if (WeckzeitErreicht()) {
      Wecken();
      }
      Es verstößt aufgrund des if gegen das IOSP, weil es sich hier nicht mehr um reine Integration handelt. Besser wäre:

      void WennWeckzeitErreicht(Action onWecken) { … }

      WennWeckzeitErreicht(Wecken);

      So ist das if in die Methode WennWeckzeitErreicht gewandert.

      Antworten
  3. durch das Herausziehen des Codes in die Methode Wecken() entsteht ein neues potentielles Problem, das vorher nicht da war: in der Ausgangsversion is das Abspielen des Sounds durch die Bedingung ‚geschützt‘ – es gibt nur den Weg durch die If-Abfrage. Nach dem Umbau kann jeder, der die Methode Wecken() findet, den Sound abspielen.
    Das *kann* manchmal praktisch sein – muss es aber nicht immer…
    Und Wecken() private machen hilft nur, wenn auch dran steht, dass es private sein soll bzw. muss.

    ich hab gesamten Code des Weckers noch nicht studiert (noch nicht gefunden) aber beim ersten Blick auf den Code fällt mir auf, das nicht zu sehen ist, wie das dauernde Wiederholen des Weckens verhindert wird. Sofort nachdem der Wecker angesprochen hat, muss die Weckzeit vorgesetzt werden.

    Zum Beispiel des Calculators möchte ich anmerken, das die Summe des Inhalts des bei Konstruktion übergebenen int[] zum Zeitpunkt des Aufrufs von Summe() gebildet wird. Am Konstruktor *muss* ein Kommentar angebracht werden, der den Aufrufer darauf hinweist, welche Folgen es hat, wenn er das übergebene Array nachträglich ändert. Vielleicht *soll* es ja so sein – dann ist der gleiche Kommentar wichtig für den späteren Leser, damit er nicht auf die Idee kommt, den Code zu ändern, um schon im Konstruktor die Summe zu bilden oder eine Kopie des Arrays anzulegen.
    Ausserdem fehlt ein Hinweis, das das Ergebnis von Summe() nur brauchbar ist, wenn die Länge des Arrays und/oder die Inhalte im ’normalen Bereich‘ liegen – wenn unterwegs ein int-Überlauf stattfindet, merkt der Aufrufer (erstmal) nichts…

    Antworten
  4. @DvH: dass nun jemand die Wecken Methode aufrufen kann ist richtig. Und wenn sie private ist, kann zumindest extern niemand die Methode aufrufen. Innerhalb der Klasse kann sie aufgerufen werden, auch hier stimme ich technisch gesehen zu. Allerdings: den Aufruf zu verhindern, in dem der Code in der urpsrünglichen Methode stehen bleibt, trennt die Aspekte nicht. Und schlimmer noch: das Argument auf die Spitze getrieben würde dazu führen, dass es am Ende nur noch eine Main Methode gibt, in der ALLES drin steht. So kann niemand andere Methoden unabsichtlich aufrufen. Übertrieben, klar… es geht immer darum, Prinzipien abzuwägen.

    Ein int[] wird in .NET als Call-by-value übergeben. Wenn also jemand das Array nachträglich ändert, DARF er nicht erwarten, dass sich die Summe bei erneutem Aufruf ändert. Insofern ist der von Ihnen vorgeschlagene Kommentar nicht erfoderlich.

    Ja, es kann zu einem Überlauf kommen. Hier ging es darum, an einem Beispiel Refactoring Maßnahmen zu zeigen und nicht darum, eine Implementation zu zeigen für die Summierung großer Arrays.

    Antworten
  5. „Ein int[] wird in .NET als Call-by-value übergeben….“ – oha – genau deshalb sind Kommentare nützlich und wichtig. Der Programmierer (Sie) soll seine Absicht erklären – damit ein Wissender (ich) erkennen kann, ob Code und Absicht zusammenpassen – oder nicht.
    Unkommentierter Code an sich ist *immer* richtig (na gut – 0-Division vielleicht nicht). Und die Idee, den einen unkommentierten Code durch mehr unkommentierten Code (sog. Unit-Tests) im Sinne von Spezifikation abzusichern, ist absurd.
    Wer schreibt, der bleibt.

    Antworten

Schreibe einen Kommentar zu Thomas Leidl Antworten abbrechen