Clean Code Regeln zum Loslegen

In diesem Artikel stelle ich euch einige von mir ausgewählte Clean Code Richtlinien und Regeln vor. Sie sollen euch dabei helfen künftig leichter verständlichen und besser wartbaren Code zu schreiben, damit ihr euren KollegInnen das Entwickler-Leben angenehmer macht.

Im Code zum Artikel wird das Spiel Minesweeper implementiert - ihr findet den Code in GitHub:
https://github.com/elmar-brauch/minesweeper.git
Und ein passendes Online Training bei Udemy.

Namen mit Bedeutung

Regel: Verwende Namen, welche die Absicht der Variable, Methode oder Klasse verraten.

Im folgenden Code-Ausschnitt habe ich Kommentare ergänzt, um auf schlechte oder zumindest suboptimale Variablennamen hinzuweisen:

public class MineField {
    // Not optimal names: cells, field, listOfListOfCells, ...
    private List<List<Cell>> fieldRows = new ArrayList<>();
    ...
    // Bad names: x, y, r, c
    public MineField(int rows, int columns) {
    ...
    public void placeMineRandomly(int mines) {
        // Not optimal names: cells, listOfCells
        var freeCells = getFreeCells();
        ...

• Namen brauchen nicht den Klassentyp zu wiederholen, siehe z.B. listOfCells.
• Namen sollten keine Abkürzungen sein, die nicht jedem direkt offensichtlich sind.
  Daher verwendete ich rows statt x und columns statt y.
• Kommentare die Abkürzungen erklären, sollten weggelassen werden und direkt den Namen der Variablen bilden, z.B.:
  int d; // elapsed time in days
  sollte einfach durch diese Variable ersetzt werden:
  int elapsedTimeInDays;
  Weitere Beispiele mit sprechenden Namen:
  int daysSinceCreation;
  int fileAgeInDays;
• Namen sollten aussprechbar sein, damit der Code leicht vorlesbar und damit auch leichter verständlich ist. Dazu auch ein Beispiel aus Clean Code: Date genymdhms;
  Dieser Name basiert auf einem Scherz im Entwicklerteam, daher wäre folgender Name einfacher auszusprechen und damit für alle leichter verständlich: Date generationTimestamp;
  
  

Funktionen

Kurz!!!

Im Buch Refactoring von Martin Fowler wird eine Methode als Bad Smell bezeichnet, wenn sie länger als eine Bildschirmseite ist. 
Daraus folgt nicht, dass eine gute bzw. Clean Code Methode bis zu einer Bildschirmseite lang sein darf!
Daraus folgt nur, dass sie dann unbedingt refaktorisiert werden soll.

SonarLint misst die kognitive Komplexität von Methoden durch Zählen von Kontrollstrukturen, wie z.B. if. Ist die kognitive Komplexität zu hoch, weil es z.B. zu viele if Anweisungen in einer Methode gibt, empfiehlt SonarLint eine Refaktorisierung. Also sagt uns SonarLint mit anderen Worten: Je weniger Kontrollstrukturen in einer Methode sind, desto leichter ist sie zu verstehen. Und kurze Methoden haben meist auch weniger Kontrollstrukturen. 

Laut Clean Code sollten Funktionen bzw. Methoden bis zu 4 Zeilen lang sein - also wirklich so kurz wie möglich.
Wie lässt sich das am besten erreichen?
Indem man z.B. für jeden Block der Kontrollstruktur eine eigene Methode erstellt, welche die Logik in diesem Pfad enthält. Außerdem hätte bei so kurzen Methoden, jede Methode vermutlich auch nur eine Kontrollstruktur. Dazu ein kurzes Beispiel:

    private void updateStatus(Cell openedCell) {
        if (openedCell.isMine())
            endGame(GameStatus.LOSE);
        else if (this.field.isEveryFreeCellOpen())
            endGame(GameStatus.WIN);
        else
            this.status = GameStatus.ONGOING;
    }

    private void endGame(GameStatus finalStatus) {
        this.status = finalStatus;
        this.field.openAllCells();
    }

• Die updateStatus Methode prüft, ob das Minesweeper Spiel fertig ist oder noch weiterläuft (ONGOING). Wenn es fertig ist, werden alle Zellen im Spielfeld (this.field) geöffnet und der Status ist Sieg oder Niederlage.
• Leider hat die Kontrollstruktur der Methode updateStatus drei Pfade, daher sind es hier 6 Zeilen.
• Dafür wurde durch die Methode endGame sichergestellt, dass jeder Pfad der Kontrollstruktur nur eine Zeile hat. Damit sind die geschweiften Klammern im if auch unnötiger Boilerplate-Code und wir können sie weglassen. Generell sind die geschweiften Klammern in Kontrollstrukturen ein Hinweis darauf, dass man sich nicht an die Clean Code Regel hält, dass Methoden kurz sind. 
• Vor meinem Refaktoring fehlte die Methode endGame. Daher waren die geschweiften Klammern nötig  und die updateStatus Methode sah so aus:

    private void updateStatus(Cell openedCell) {
        if (openedCell.isMine()) {
            status = GameStatus.LOSE;
            field.openAllCells();
        } else if (field.isEveryFreeCellOpen()) {
            status = GameStatus.WIN;
            field.openAllCells();
        } else
            status = GameStatus.ONGOING;
    }

Nur eine Sache und sprechende Namen

Methoden sollen nur eine Sache machen. Diese Regel passt sehr gut zur vorherigen Regel, denn die Wahrscheinlichkeit, mehr als eine Sache in einer Methode zu tun, steigt je länger die Methode wird.

Im vorherigen Code-Ausschnitt setze ich den Status des Spiels und öffne alle Zellen, wenn der Status das Spiel beendet. Das sind eigentlich 2 Sachen, die in unterschiedliche Methoden getrennt werden sollten.

Wenn wir unseren Methoden sprechende Namen geben, die erklären was die Methode macht, wird der Name immer länger je mehr Sachen in einer Methode gemacht werden. Wenn wir also der Regel folgen, dass die Namen unserer Methoden beschreiben, was sie tun, bekommen wir lange Methodennamen. Sollten die Methodennamen zu lang werden ist das ein Hinweis, dass wir in der Methode zu viele Dinge gleichzeitig tun.

Ein sprechender Methodenname für updateStatus könnte also updateStatusAndOpenAllCellsIfGameEnds sein. Dieser Name beschreibt genau was die Methode macht und ist aufgrund seiner Länge ein Hinweis, dass es hier eine bessere Lösung geben sollte.

Single Responsibility Prinzip

Auf Klassen-Ebene gibt es das Single Responsibility Prinzip. Dieses besagt, dass sich Klassen nur aus einen Grund ändern dürfen. Nur wenn sich die Spezifikation der Funktionalität ändert, darf sich auch der Code in der Klasse ändern. Die Klasse darf sich aber nicht ändern, weil sich eine andere Klasse aus einem anderen Grund ändert. Damit konzentriert sich die Klasse auf eine Aufgabe und sollte dann auch einen Namen haben, der diese Aufgabe beschreibt. Die Beziehung zwischen Klasse und Spezifikation dem Single Responsibility Prinzip folgend ist hier grafisch schön gezeigt: https://petozoltan.gitbook.io/clean-code/single-responsibility-principle

Parameter Anzahl

Laut Clean Code hat eine Methode idealerweise keine Parameter.
Mit jedem Parameter wird es schwieriger die Methode beim Lesen schnell zu verstehen.
Demnach sind ein oder zwei Parameter gut, wobei ein Parameter besser ist als zwei Parameter.
Methoden mit drei Parameter sollen nach den Clean Code Richtlinien, wann immer möglich, vermieden werden.
Mehr als drei Parameter in einer Methode sollen extrem gut begründet werden. Und selbst bei einer Begründung, warum die vielen Parameter nötig sind, sollen diese Methoden nicht verwendet werden ;-)

Regel: Methoden mit mehr als 2 Parametern sollten so refaktorisiert werden, dass sie mit zwei, besser einem oder keinem Parametern auskommen.

Der folgende Methoden-Aufruf hat 2 Parameter und einen nicht ganz klaren Namen:
        changeAllCellsInToOpen(field, true);

Nach Analyse des Code lernen wir, dass sich alle Zellen im Parameter field verbergen und true der neue Wert für das Attribute open jeder Zelle ist. Als Entwickler des Codes weiß ich, dass ich Zellen nur öffne und nie schließe, daher ist der 2. Parameter eigentlich unnötig, so dass ich durch Anpassung der Methode folgenden Aufruf ermöglichen kann:
        openAllCellsIn(field);

Und wenn wir uns jetzt noch auf die Prinzipien der Objekt-Orientierung besinnen, stellen wir fest, dass die Methode openAllCellsIn eigentlich auch direkt auf dem Objekt field aufgerufen werden kann. Dazu muss die zuvor statische Methode in die Klasse MineField (der Typ von field) verschoben werden und dann kommen wir sogar ohne Parameter aus:
    field.openAllCells();

JUnit bietet ein weiteres Beispiel für kompliziert zu verstehende Methoden:
...void assertEquals(Object expected, Object actual, String message)
Wenn wir mit dieser Methode Strings vergleichen, wissen wir häufig nicht direkt, dass der erste Parameter der erwartete Wert ist, der zweite der aktuelle und der dritte die Nachricht, die beim Testfehlschlag angezeigt wird. Es könnte also passieren, dass wir aus versehen den expected Parameter mit message verwechsel. Im Vergleich dazu ist die assertTrue oder assertFalse Methode von JUnit deutlich leichter richtig zu verwenden, da sie nur einen Parameter hat und den kann man nicht vertauschen.

Kein Copy & Paste Code!

Die Regel produziere keinen Copy & Paste Code in Deinem Projekt, ist beim Programmieren für mich so etwas wie eines der 10 Gebote von Moses. Ich hinterfrage nicht das Gebot "Du sollst nicht stehlen." und analog handle ich immer nach der Regel "Don't repeat yourself" bzw. kein Copy & Paste Code. Für EntwicklerInnen, die sich mit Clean Code befassen, ist diese Regel offensichtlich - für alle andere: Jede Code Duplikation verschlechtert die Wartbarkeit des Codes.

Ich bezweifle, dass es gute Software EntwicklerInnen gibt, die das Gegenteil behaupten. Hier ist aber noch eine Liste von berühmten Entwicklern, die das "Don't repeat yourself" Gebot aufstellten oder in ihren Büchern ausführlich erklären:

• Robert Cecil Martin (Autor von Clean Code): No Duplication - "most important rule[s] in this [Clean Code] book". 
• David Thomas & Andy Hunt (The Pragmatic Programmer): "Don't repeat yourself"
• Kent Beck nennt eines der Kernprinzipien vom Extreme Programming: "Once, and only once."
• Ron Jeffries (Autor des Agile Manifesto) "ranks [No Duplication] rule second, just behind getting all tests to pass"

Diese Regel betrifft übrigens nicht nur euren Sourcecode, sie betrifft alles. Also zum Beispiel auch Konfigurations-Dateien (application.properties), die Maven Build Konfiguration (pom.xml), den CICD Pipeline Code, html Seiten bzw. ihre Templates, CSS Dateien usw. Für (fast) alle Technologien gibt es wiederverwendbare Templates, Funktionen, Includes, etc. also benutzt das und vermeidet Duplikationen bzw. Copy & Paste Code!

Kommentare

Kommentare sind ein zweischneidiges Schwert:

• An der richtigen Stelle, gut und passend geschrieben helfen sie.
• Häufig sind sie aber veraltet, falsch oder unnötig, so dass sie nur stören.

Robert C. Martin bezeichnet Kommentare immer als Fehler, weil es dem Entwickler nicht gelungen ist sich mit sauberem, leicht verständlichen Code so auszudrücken, dass der Kommentar nicht nötig ist.

Besonders kritisch sind Kommentare, die dem Code widersprechen. In der Regel sind das veraltete Kommentare, die bei Code-Refaktorisierungen nicht mit angepasst wurden. Die Refaktorisierungsfunktionalitäten unserer IDEs ändern automatisch den Code an allen betroffenen Stellen, Kommentare können sie aber nicht neu schreiben. Das kann dann die Ursache eines widersprüchlichen Kommentars sein, der uns LeserInnen des Codes vor die Frage stellt: "Was ist richtig - Code oder Kommentar?" Dazu ein Beispiel:

/**

  * Plays one round of the game, if parameter position is given

  * otherwise the game is restarted. 

  */

@PostMapping

public String openCell(Model model,
        @RequestParam(required = false) String position, ...

In diesem Kommentar wird erklärt, was die Methode openCell macht. Im Rahmen einer Refaktorisierung wird die Methode in 2 Methoden aufgeteilt:

/**

  * Plays one round of the game, if parameter position is given

  * otherwise the game is restarted. 

  */

@PostMapping

pubic String openCellToPlayOneRound(...
...

@PostMapping

public String restartGame(...

Bei diesem Refactoring wurde das Anpassen des Kommentars vergessen, so dass wir nun den Widerspruch haben, dass die Methode openCellToPlayOneRound das Spiel nie neustarten wird.

Um das zu verhindern, sollten wir uns vor dem Schreiben eines Kommentars zuerst fragen, ob wir die anderen Regeln für Clean Code beachtet haben und ob der Kommentar wirklich nötig ist. Können wir z.B. den Kommentar an einer Methode weglassen, indem wir die Methode verkleinern, ihr einen sprechenden Namen geben und die Anzahl der Parameter reduzieren? Eigentlich sollte dass fast immer möglich sein, so dass der Kommentar überflüssig wird.

An manchen Stellen sind Kommentare sinnvoll. Wenn ihr zum Beispiel eine öffentlich Bibliothek erstellt habt und diese von anderen Teams genutzt wird, ist JavaDoc eine gute Wahl, um die Interfaces und public Methoden in der Bibliothek zu beschreiben.

Fehlerbehandlung

Richtlinie: Verwende Exceptions anstatt Fehlercodes.

Mittlerweile sollten sich in allen beliebten Bibliotheken Exceptions anstelle von der Rückgabe von Fehlercodes zur Fehlerbeschreibung durchgesetzt haben. Wenn ihr eine Funktion verwendet bei der etwas schief gehen kann, müsstet ihr im Rahmen der Fehlerbehandlung jeden möglichen Fehlercode einzeln prüfen und würdet so den Code auf der Client-Seite verschmutzen. 

Um das zu verhindern wurden Exceptions eingeführt. Ihr ruft die betroffene Funktion innerhalb eines try-catch-Blocks auf und arbeitet den Erfolgsfall einfach innerhalb des try-Blocks ab, während alle Exceptions in den catch-Blöcken verarbeitet werden. Sollten tatsächlich Fehlercodes nötig sein, könnt ihr diese auch einfach als Attribute eurer Exceptions definieren. Hier ein Beispiel aus dem Spring Framework:

public class WebClientResponseException extends WebClientException {

    ...

    private final int statusCode;

    private final String statusText;

    private final byte[] responseBody;

    private final HttpHeaders headers;

    ...

• Wenn ihr mittels Spring WebClient HTTP Requests verschickt, bekommt ihr im Erfolgsfall einfach die erwartete Antwort. Sollte es zu einem Fehler kommen, bekommt ihr eine WebClientResponseException.
• Mögliche Fehlercodes orientieren sich an den HTTP Status Codes der Response. Dafür sind die Attribute statusCode und statusText in WebClientResponseException da. Fehlercodes als mögliche Antwort im Erfolgsfall sind also nicht nötig.
  
  

Richtlinie: Verwende unchecked Exceptions anstatt checked Exceptions.

Die WebClientException aus dem vorherigen Code-Ausschnitt ist übrigens eine Unterklasse von RuntimeException. RuntimeException ist in Java die Oberklasse der Unchecked Exceptions, also einer Exception deren Fangen der Compiler nicht erzwingt. Aus anderen Programmiersprachen wie zum Beispiel C#, Python und  Kotlin wissen wir, dass Checked Exceptions nicht nötig sind, um robuste Software zu schreiben. 

In der Praxis finden wir in vielen Java Programmen gefangene Exceptions, die keinerlei besondere Behandlung haben (außer evtl. loggen), da eine besondere Behandlung nicht möglich ist. Dafür ist unser Code an vielen Stellen durch try-catch-Blöcke aufgebläht oder unsere Methoden Signaturen definieren alle Exceptions mit throws. Das Problem dabei ist, dass Software häufig in Schichten strukturiert ist und wenn wir in der untersten Schicht eine Checked Exception werfen, müssen die Signaturen der aufrufenden Methoden in allen Schichten auch angepasst werden.

• Deshalb verwendet Spring beim WebClient unchecked Exceptions.
• Deshalb gibt es in Kotlin nur unchecked Exceptions, weil man aus dem Design-Fehler Checked Exception von Java gelernt hat.
• Deshalb solltet auch ihr in eurem Code unchecked Exceptions bevorzugen.
  
  

Null der Billion-Dollar-Fehler

Null wurde 1964 von Tony Hoare erfunden. Später bezeichnete er die null Referenz als seinen Billion-Dollar-Fehler. Wir alle kennen NullPointerExceptions. Der einzige Weg, um sie sicher in Java zu verhindern, ist null niemals als Rückgabe zu verwenden.

Wie können wir den Rückgabewert null verhindern?

• Wenn unsere Rückgabe im Normalfall eine Collection ist, geben wir im "null"-Fall besser eine leere Collections, Liste oder Map statt null zurück. 
• In Fehlerfällen werfen wir eine passende Exception.
• Ansonsten verwenden wir Optionals für unsere Rückgabe. Die Klasse Optional wurde mit Java 8 eingeführt, um null als Rückgabewert überflüssig zu machen. Dazu folgender Code-Ausschnitt:

    public void printText() {

        findText().ifPresent(System.out::println);

    }

    public Optional<String> findText() {

        String text = service.returnsNullOrText();

        return Optional.ofNullable(text);

    }

• returnsNullOrText ist eine nicht von uns geschriebene Methode, die leider null zurück geben kann. Weil sie null zurück gibt müssen wir einen null-Check machen. Damit aber nach uns niemand mehr einen null-Check machen muss, geben wir einen Optional zurück.
  Die Klasse Optional hat praktischer Weise schon einen integrierten null-Check in der Methode ofNullable, so dass entweder ein Optional mit dem übergebenen Objekt (Optional.of(text)) oder ein leeres Optional (Optional.empty()) zurück gegeben wird.
• Der Nutzer unserer Methode sieht, dass wir einen Optional zurückgeben und verlässt sich damit darauf, dass wir nie null zurück geben. Der Client muss also keinen null-Check mehr machen und kann einfach den Optional verwenden.
• Modernere Konzepte in Java, wie z.B. die Stream API, sind ebenfalls so aufgebaut, dass sie nie null zurück geben, sondern Optionals verwenden.
  

Hier noch ein gutes Video, das ich auf YouTube gefunden habe: Niemals Null zurückgeben

Pfadfinder-Fazit

Die hier vorgestellten Punkte stammen aus dem Buch Clean Code von Robert C. Martin. Das Buch ist herausragend gut und eine klare Lese-Empfehlung für jede ProgrammiererInnen.

All die genannten Punkte helfen euch besseren Code zu schreiben. Doch wo soll man anfangen, wenn man im Legacy-System von Altlasten erdrückt wird oder einige Kollegen hartnäckig die Regeln für Clean Code ignorieren?

Fang ab jetzt sofort bei Deinem eigenen Code an und halte Dich dabei an die Pfadfinder-Regel:
"Hinterlasse einen Ort immer in einem besseren Zustand als du ihn vorgefunden hast".
Wenn Du Methoden, Klassen oder Codes, die von anderen geschrieben wurden, anpassen musst, wende dabei die Clean Code Regeln an, damit der nächste Entwickler diese Methoden, Klassen oder Codes in einem besseren Zustand findet als Du.