In einem hübschen Artikel hat Mark Nijhof ein Refactoring nach SOLID beschrieben. Dem ist nichts hinzuzufügen - wenn man denn bei SOLID stehen bleiben will. Ich sehe SOLID aber nicht als sakrosankt an; für mich darf Code gern noch "cleaner" werden.
Hier zunächst die Code-Ausgangslage:
Eine Klasse mit eine Menge Verantwortungen. Mark löst sie im Sinne des Single Responsibility Principle (SRP) wie folgt auf:
Dagegen ist nichts einzuwenden. Es entstehen drei "Akteure", d.h. Funktionseinheiten mit jeweils wiederum potenziell vielen Aufgaben. Hinweis darauf ist die Benennung mit Substantiven. Für den Moment hat Mark zwar Verantwortlichkeiten getrennt - doch das ist "Rückspiegelweisheit": Bei Vorlage einer Brownfield-Klasse ist ihm die Vermengung von Verantwortlichkeiten in einer Klasse aufgefallen.
Wäre es aber nicht viel besser, bei einer Refaktorisierung Code zu erzeugen, der einer solchen Entwicklung von vornherein Widerstand leistet? Vorbeugen ist besser als Refactoring. Dazu bedarf es jedoch einer Idee von der Ursache der Verantwortungsanhäufung. Woher könnte die rühren?
Ich glaube, das hat nicht nur mit Unaufmerksamkeit der Entwickler zu tun, die zum ursprünglichen Stand von OrderProcessor beigetragen haben. Mitschuldig ist auch die Bennung der Klasse. Sie ist als Akteur ausgelegt, als Substantiv, d.h. als Funktionseinheit, die qua Name suggeriert "Stopfe mich voll mit allem, was mit der Auftragsverarbeitung zu tun hat."
So machen wir es doch auch, wenn ein Mensch als Akteur vor uns steht. "Ach, kannst du das nicht auch noch machen?" Und schon hat er eine Verantwortlichkeit mehr am Hacken.
Um das Wurzelproblem anzupacken, fände ich es besser, die Namen anders zu wählen. Allemal die der Logik-Funktionseinheit. Sie sollte nicht OrderProcessor heißen, sondern Process_order. Denn nur darum geht es hier. Noch schöner wäre es, wenn dazu dann Send_confirmation_email und Save_order dazu kämen:
Lassen Sie Ihr Unwohlsein angesichts der merkwürdigen Methodenbezeichnung "_" kurz beiseite. Spüren Sie stattdessen einmal in sich hinein: Wir groß ist nun die Gefahr, dass Process_order oder eine der beiden anderen Klassen mit weiterer Funktionalität aufgeladen wird, die nichts direkt mit dem Klassennamen zu tun hat? Für mich ist da jetzt eine spürbare Barriere.
Nun aber zu einem noch wichtigeren Aspekt im wahrsten Sinn des Wortes, den Mark bei seinem Separierungsbestreben übersehen hat. Er ist schwer zu sehen, weil wir alle so traditionell OO-konditioniert sind. Wir halten den Code in dieser Hinsicht für normal. "So macht man das halt" denken wir und hat Mark gedacht.
Mit geht es um das, was in Process_order passiert. Ich greife es mal heraus:
Wieviele Verantwortlichkeiten hat Process()? Wieviele Gründe für Veränderung gibt es?
- Muss die Methode "angefasst werden", wenn die Bestätigungen per Fax statt der Email versandt werden sollen?
- Muss die Methode verändert werden, falls nach der Email-Bestätigung auch noch ein Eintrag in ein Protokoll gemacht werden soll?
- Muss die Methode überarbeitet werden, wenn sich die Bedingung für eine Email-Bestätigung verändert?
Die Antwort auf alle drei (!) Fragen ist Ja. Die Methode hat mithin nicht nur eine Verantwortlichkeit. Die Fragen gehören nämlich zu unterschiedlichen Verantwortlichkeitsbereichen:
- Implementationsauswahl
- Integration
- Kontrolle
Die Methode kontrolliert die Integration konkreter Implementationen für Operationen. Sie stellt also nicht nur sicher, dass bestimmte Operationen in einer bestimmten Reihenfolge ablaufen, nein, sie entscheidet auch noch über diese Reihenfolge zur Laufzeit und instanziiert ganz bestimmte Ausprägungen der Operationen.
Zur Marks Ehrenrettung sei gesagt, dass er eine dieser Verantwortlichkeiten auch durch seine SOLID-Brille erkannt hat: die Implementationsauswahl. Sie löst er über Inversion of Control auf. (Dass er später auch noch einen DI Container einsetzt, ist nicht so wichtig.)
Also mache ich meine Code-Version auch IoC-konform:
Dabei bleibt es dann bei Mark. Er belässt die Verantwortlichkeiten Integration und (!) Kontrolle in der Methode. Er erkennt sie schlicht nicht. Sie liegen außerhalb seines Wahrnehmungshorizonts.
Dabei ist es so einfach: Wo eine Kontrollstruktur - nomen est omen - wie if oder for im Spiel ist, da werden Operationen nicht nur "hintereinandergeschaltet", sondern auch darüber entschieden, wann die unterschiedlichen Pfade durchlaufen werden. Also muss man ein Augenmerk darauf haben, dass diese Entscheidung nicht ebenfalls in derselben Methode gefällt wird. Darum geht es beim Prinzip Single Level of Abstraction (SLA) - das allerdings nicht zu SOLID gehört. Schade.
Um konsequent SRP umzusetzen, muss die Bedingung, unter der eine Bestätigung gesendet werden soll, raus aus der Auftragsverarbeitung. Das kann zunächst in einfacher Weise geschehen:
Aber fühlt sich das wirklich gut an? Mich schüttelt es, weil nun ganz deutlich wird, dass die Bedingung zwei Verantwortlichkeiten hat. Nicht umsonst musste ich einen Namen wählen, der eine Konjunktion enthält. Nur Is_order_valid() hätte die Speicherung unterschlagen, nur Successfully_saved_order() hätte die Validation unterschlagen.
Das Ziel ist richtig, die Verantwortlichkeiten zu entzerren. Aber das Mittel ist falsch. Besser finde ich es so:
Jetzt ist deutlicher und ohne Schachtelung die Sequenz des Ablaufs zu sehen:
- Gültigkeit des Auftrags prüfen
- Auftrag speichern
- Email-Bestätigung senden
Hier könnte ich es gut sein lassen. Doch ich bin seit der Lektüre von "Clean Code" sensibel geworden. Die Verantwortungshäufung lauert überall. Vor allem lädt der obigen Code bei aller Sauberkeit immer noch (oder wieder) dazu ein, in ihm weitere Verantwortungen anzuhäufen. Wie schnell ist die Validitätsbedingung aufgebohrt und sieht dann z.B. so aus:
Schon wieder steckt die Kontrollverantwortung mit drin.
Und warum? Weil die Kontrollstruktur if in der Methode geblieben ist. Sie ist zwar relativ harmlos, solange sie nur eines tut, nämlich den Codefluss entsprechend eines Flags mal in dieser und mal in jener Richtung zu leiten. Doch wie ein Stäubchen in der Luft ist sie ein Kristallisationskeim: für Domänenlogik. Eben noch steckte die ausschließlich in IsValid, doch dann hat die sich ausgebreitet, weil grad keine Zeit war, über eine Methode auf Order oder sonst wo nachzudenken, die beide Bedingungsklauseln umfasst. Ja, so schnell kann es gehen. So entsteht Entropie (oder dirty code) in kleinsten Inkrementen.
Hört sich vielleicht nach Erbsenzählerei an. Mag sein. Aber wenn ein Container voller Erbsen zerbricht, kann das ordentlichen Schaden anrichten.
Ich meine also, dass die Verantwortlichkeit Integration solange nicht ordentlich herauspräpariert ist, wie in einer Methode noch Kontrollstrukturen stehen. Aber wie kann die Integration der drei Operation zu einem Ganzen - der Auftragsverarbeitung - erreicht werden, wenn sie einerseits von Bedingungen abhängig ist, andererseits jedoch keine Kontrollstrukturen dafür enthalten darf?
Die verblüffend einfache Antwortet lautet: mit Continuations. (Mit Erweiterungsmethoden ginge es auch. Aber damit würden wir uns auf statische Methoden festlegen.)
Meine Version der Refaktorisierung mit Continuations so aus:
Die Methode zur Auftragsverarbeitung ist jetzt ausschließlich für die Integration von Operationen im Sinne eines Verarbeitungsflusses zuständig. Und sie lädt niemanden mehr ein, Logik hineinzustecken. (Naja, wer will, der bohrt natürlich die Lambda-Ausdrücke auf. Aber das halte ich für weniger naheliegend als mal eben eine Bedingung zu erweitern.)
Validate_order() ist ebenfalls auf eine Verantwortlichkeit konzentriert: Kontrolle. Wer die Validitätsbedingung verändern will, ist dort genau richtig. Was anderes kann man dort aber auch nicht sinnvoll tun. Insofern muss der Ausdruck auch nicht in eine weitere Methode ausgelagert werden.
Zum Schluss noch zu den merkwürdig benannten Methoden der zum Speichern und Senden des Auftrags. Ich habe die Namen auf einen Unterstrich beschränkt, um den Code besser lesbar zu halten. Oder ist _save._() nicht besser zu lesen als _save.Process() oder _save.Save()?
Dennoch verstehe ich, wenn Sie bei "_" als Methodenname zucken. Er scheint so nichtssagend. Klar. Er muss ja auch nichts sagen, weil der Klassenname (und der davon abgeleitete Feldname) alles sagt.
Fallen Sie angesichts Ihres Unwohlseins nun aber nicht zurück in alte Gewohnheit. Prügeln Sie sich aber auch nicht, "_" als Methodenname zu akzeptieren. Sondern machen Sie den nächsten konsequenten Schritt, den Mark ebenfalls nicht vollzogen hat. Den Robert C. Martin sich vor ihm nicht getraut hat.
Denken Sie das Interface Segregation Principle (ISP) weiter.
In Marks Artikel kommt das ISP in Bezug auf den Ausgangscode eigentlich gar nicht zum Einsatz. Er muss dafür weitere Einsatzszenarien erfinden. Das liegt daran, dass das ISP eben ein I-SP ist; es beharrt darauf, Dienstleistungen in Bündeln anzubieten und zu nutzen. Das IoC und der DI Container arbeiten auf Interfaces, obwohl im Beispiel von jedem Interface nur 1 Methode gebraucht wird.
Das ist nicht nur hier, sondern sehr oft der Fall. Integrationen bekommen Akteure mit vielen Dienstleistungen hineingereicht, nutzen davon aber nur sehr wenige, oft nur 1 oder 2. Warum also überhaupt Abhängigkeiten auf Interfaces basieren? Der Code könnte ohne auskommen, ohne IoC und DI Container zu vernachlässigen:
Voilà! Die Unterstriche sind verschwunden. Interfaces sind nicht mehr nötig. Process_order ist von keiner Implementation abhängig. Process_order.Process() hat weiterhin nur eine Verantwortlichkeit.
Fazit
SOLID ist gut. SOLID+SLA ist besser. Und SOLID+SLA+ISP2TheMax ist noch besser.
SOLID hat uns auf dem Weg zu Evolvierbarkeit voran gebracht. Wir sollten uns nun aber nicht ausruhen. Das Ziel ist nicht erreicht. Deshalb dard es gern noch konsequenter prinzipieller zugehen. Schauen Sie genau hin, hinterfragen Sie das Selbstverständliche und Überkommene. Die Kristallisationskeime für Dreck sind manchmal klitzeklein. Über die Zeit entfalten sie nichtsdestotrotz eine ungute Wirkung. Versuchen Sie deshalb, Code schon beim Schreiben in der Form so zu halten, dass an ihm Dreck nicht anhaften kann.
Continuations statt Kontrollstrukturen und Delegaten statt Interfaces sind ein Anfang.
Eine Technik zur Vermeidung von dreckigem Code von vornherein ist besser als eine Prinzip zum Aufspüren von Dreck im Code im Nachhinein.
Spendieren Sie mir doch einen Kaffee, wenn Ihnen dieser Artikel gefallen hat...