Posts
Clean Code am Minesweeper Beispiel
Mit Clean Code ist man nie fertig... Als ich mir den Code zu meinem vorherigen Clean Code Artikel ein zweites und drittes Mal angeschaut habe, habe ich immer wieder neue Punkte gefunden, die nicht so richtig sauber waren. Deshalb möchte ich in diesem Artikel weitere Beispiele für Clean Code anhand meiner Minesweeper Demo zeigen.
Im Code zum Artikel wird das Spiel Minesweeper implementiert, die Regeln findet ihr hier.
Den kompletten Code von mir findet ihr in GitHub:
https://github.com/elmar-brauch/minesweeper.git
Und ein passendes Online-Training bei Udemy.
Verzicht auf (unnötige) Setter
Häufig generieren wir für alle Attribute unserer Klassen Getter und Setter Methoden. Das hat zum Beispiel den Nachteil, dass wir ungenutzte und damit unnötige Attribute in unserer Klasse nicht erkennen, da sie vom Getter und Setter genutzt werden.
In der Klasse Cell des Minesweeper Spiel ist mir aber noch ein 2. Nachteil aufgefallen. Aus Clean Code wissen wir, dass die optimale Methode keine Parameter hat. Setter haben aber einen Parameter. In meiner ersten Minesweeper Version hatte die Klasse Cell das Boolean Attribut open, um anzuzeigen, ob die Zelle bereits geöffnet ist oder nicht. Mittels Setter habe ich dann nach jedem Klick auf eine Zelle den Zustand auf open == true gesetzt:
public class Cell {
private boolean open;
public void setOpen(boolean open) { this.open = open; }
public boolean isOpen() { return this.open; }
...
...
Mit dem Setter kann man die Zelle öffnen und schließen. Das macht beim Spiel Minesweeper aber keinen Sinn, da der Spieler nur geschlossene Zellen anklicken und damit einmalig öffnen kann. Eine geöffnete Zelle kann nicht mehr geschlossen werden.
Wir können also auf den Setter verzichten und stattdessen eine Methode open implementieren, die bei Aufruf den Status der Zelle auf geöffnet setzt. Als Fan der Lombok-Bibliothek, schreibe ich meine Getter und Setter so wie so nicht explizit, sondern mache das per Annotation:
@Getter
public class Cell {
private boolean open = false;
public void open() { this.open = true; }
Damit habe ich eine Methode mit optimaler Parameteranzahl 0 und gleichzeitig eine Klasse Cell, die besser die Fachlichkeit des Spiels repräsentiert. Also 2 Vorteile durch den Verzicht auf Setter.
Das Ersetzen von Setter-Methoden durch parameterlose Methoden funktioniert immer dann gut, wenn das zu setzende Attribute nur wenige mögliche Werte hat. Das kommt bei einem Boolean- oder Enum-Parameter deutlich häufiger vor als bei einem String- oder Klassen-Parameter.
Überfällige Klassen einführen
Manchmal zögere ich beim Erstellen von neuen Klassen, weil es schon eine Klasse gibt, die die Logik irgendwie abdeckt. Dieses Verhalten behindert das Schreiben von sauberem Code. In meiner ersten Minesweeper Version habe ich die Position der Zelle im Minenfeld mit Attributen in der Klasse Cell beschrieben:
public class Cell {
private final int inRow;
private final int inColumn;
...
public String getPositionAsString() {
return inRow + ":" + inColumn;
}
...
Um die Position mit dem UI auszutauschen, habe ich mich für ein String-Format entschieden, welches die Reihe mit Doppelpunkt von der Spalte trennt, siehe getPosition. An dieser Stelle hätte ich eigentlich schon sagen müssen, die Position der Zelle ist eine eigene Klasse, leider habe ich noch etwas gezögert, was dazu geführt hat, dass ich an einer komplett anderen Stelle in der Klasse GameController noch einen Parser für die Position schreiben musste:
public class GameController {
...
@PostMapping("play")
public String openCellToPlayOneRound(
@RequestParam String position) {
var rowAndCol = position.split(":");
game.playRound(
Integer.parseInt(rowAndCol[0]),
Integer.parseInt(rowAndCol[1]));
...
Das war dann für mich der Trigger, um die überfällige Klasse Position einzuführen und somit insbesondere die Logik zum Serialisieren und Parsen der Position im Minenfeld an der richtigen Stelle in derselben Klasse zu haben:
public class Position {
private final int row;
private final int column;
public static Position parse(String positionAsString) {
var rowAndCol = positionAsString.split(":");
return new Position(
Integer.parseInt(rowAndCol[0]),
Integer.parseInt(rowAndCol[1]));
}
@Override
public String toString() {
return row + ":" + column;
}
...
Die Klassen Cell wird durch Position einfacher, da sie statt 2 Attributen nur noch 1 benötigt. Außerdem kann auf getPositionAsString verzichten werden, da diese Methode durch Position.toString ersetzt wird. Im GameController entfällt die Logik zum Parsen, so dass die Klasse entsprechend einfacher und leichter verständlich wird:
public class GameController {
...
@PostMapping("play")
public String openCellToPlayOneRound(
@RequestParam String position) {
game.playRound(Position.parse(position));
...
Rekursion, die elegante Schleifenalternative
Als ich im Informatik-Studium zum ersten Mal die Rekursion kennengelernt habe, war das ziemlich kompliziert. Die klassischen Schleifen (Iteration) hatte ich bereits verinnerlicht und nun sollte ich Schleifen durch Methoden ersetzen, die sich selbst aufrufen. Detaillierte Erklärungen zur Rekursion gibt es im Informatik-Studium oder hier: https://www.java-tutorial.org/iteration_und_rekursion.html
Wenn wir über Clean Code schreiben wollen, wollen wir häufig so wenig Code wie nötig schreiben und dabei kann uns das Konzept der Rekursion helfen. Als Beispiel habe ich die Methode openCell aus Minesweeper mitgebracht. Wenn der Wert der geöffneten Zelle 0 ist bzw. wenn sich in keinem benachbarten Feld eine Mine befinden, dann soll diese Methode automatisch alle Nachbarzellen öffnen. Haben diese ebenfalls keine Minen in angrenzenden Felder, werden erneut die benachbarten Zellen geöffnet und so weiter. Dazu die iterative Implementierung mit while-Schleife:
public void openCell(Position position) {
var cells2Open = new ArrayList<Cell>();
cells2Open.add(getCellAtPosition(position).orElseThrow());
while (!cells2Open.isEmpty()) {
var cell2Open = cells2Open.remove(0);
cell2Open.open();
if (CellStatus.AWAY_OF_MINES.equals(cell2Open.getStatus())){
var closedNeighbours =
getNeighbourCells(cellToOpen.getPosition())
getNeighbourCells(cellToOpen.getPosition())
.stream()
.filter(c -> !c.isOpen())
.filter(c -> !cellsToOpen.contains(c))
.toList();
cellsToOpen.addAll(closedNeighbours);
}
}
}
Wenn wir ausnahmsweise ignorieren, dass der Anweisungsblock innerhalb der while-Schleife oder des if-Ausdrucks nach Clean Code Regeln eigentlich in eine separate Methode gehört, dann ist das eine relativ kompakte Implementierung dieser Logik. In der ArrayList cells2Open sammle ich alle Nachbarzelle, die automatisch geöffnet werden sollen. Im Anweisungsblock der Schleife verarbeite ich immer eine Zelle aus der Liste cells2Open, indem ich die Zelle öffne und falls sie im Status AWAY_OF_MINES ist, werden ihre geschlossenen und nicht bereits enthaltenen Nachbarzellen der Liste cells2Open hinzugefügt. Man kann also sagen, die Methode ist so kompliziert, wie sie sich erklären lässt und das ist nicht im Einklang mit Clean Code.
Sofern ihr die Rekursion bereits verinnerlicht habt, hättet ihr diese Methode vermutlich nie iterativ geschrieben. Der rekursiven Variante reichen 4 Anweisungen und sie sieht so aus:
public void openCell(Position position) {
var cell2Open = getCellAtPosition(position).orElseThrow();
cell2Open.open();
if (CellStatus.AWAY_OF_MINES.equals(cell2Open.getStatus())){
getNeighbourCells(position).stream()
.filter(c -> !c.isOpen())
.map(Cell::getPosition)
.forEach(this::openCell);
}
}
Wie bereits in der iterativen Variante muss die Zelle cell2Open geöffnet werden und ihr Status muss auf AWAY_OF_MINES überprüft werden. Wenn die Zelle neben keiner Mine liegt, werden alle geschlossenen Nachbarzelle wie zuvor mit getNeighbourCells ermittelt und für jede wird die Methode openCell erneut aufgerufen. Das ist die Rekursion und in diesem Fall die elegante Schleifenalternative!
Programm-Logik überdenken
In meinem ersten Clean Code Artikel habe ich erklärt, dass Methoden kurz sein sollen und es dennoch nicht geschafft mit einfachen Refaktorisierungen die folgende Methode auf maximal 4 Zeilen Code zu kürzen:
private void updateStatus(Cell openedCell) {
if (openedCell.isMine())
endGame(GameStatus.LOSE);
else if (this.field.isEveryFreeCellOpen())
endGame(GameStatus.WIN);
else
this.status = GameStatus.ONGOING;
}
if (openedCell.isMine())
endGame(GameStatus.LOSE);
else if (this.field.isEveryFreeCellOpen())
endGame(GameStatus.WIN);
else
this.status = GameStatus.ONGOING;
}
Das Problem an dieser Stelle ist, dass ich in der Methode zwischen 3 möglichen Status den richtigen finden muss. Beim Review der Programm-Logik ist mir dann aber aufgefallen, dass es im Minesweeper Spiel eigentlich nur 2 mögliche Status-Wechsel gibt:
- Das Spiel ist ONGOING (frisch gestartet oder läuft) und endet mit dem Status WIN
- Das Spiel ist ONGOING und endet mit dem Status LOSE
Ob das Spiel frisch gestartet ist oder schon läuft interessiert nicht, da es den Status "frisch gestartet" auch nicht im enum GameStatus gibt. Demnach ist es nie notwendig einen Wechsel von irgendeinem beliebigen Status in den Status ONGOING zu machen. Wenn das Spiel neu gestartet wird, wird eine neue Instanz der Klasse Game erzeugt, welche den initialen Status ONGOING hat. Damit ist mir beim Überdenken der Programm-Logik aufgefallen, dass es eine noch einfachere und saubere Implementierung dieser Methode gibt. Dieser habe ich dann auch noch einen passenderen Namen gegeben:
private void checkEndOfGame(Cell openedCell) {
if (openedCell.isMine())
endGame(GameStatus.LOSE);
else if (this.field.isEveryFreeCellOpen())
endGame(GameStatus.WIN);
}
if (openedCell.isMine())
endGame(GameStatus.LOSE);
else if (this.field.isEveryFreeCellOpen())
endGame(GameStatus.WIN);
}
Fazit
Macht es Sinn geschriebenen Code mehrfach zu reviewen, um wie hier gezeigt die (vorläufig) letzten bzw. "besten" Vereinfachungen zu finden und umzusetzen? Diese Frage ist nicht einfach zu beantworten, da es auf den Kontext ankommt. Meist wird mehr Zeit von unterschiedlichen EntwicklerInnen zum Lesen und Verstehen eines Codes benötigt als der Urheber des Codes benötigt hat ihn zu schreiben. In diesem Fall ist es ein lohnendes Investment den Code so einfach wie möglich zu schreiben. Bei Ein-Personen-Teams sieht es evtl. anders aus, da außer dem Urheber niemand (in absehbarer Zeit) den Code liest.
Ein anderer Aspekt ist die Frage, wie werden wir als EntwicklerInnen besser und lernen sauberen Code zu schreiben? Je öfter wir es bewusst tun, so wie hier gezeigt, desto besser werden wir darin direkt möglichst sauberen Code zu schreiben. Nur Erfahrung sammeln beim Programmieren hilft uns da nicht weiter. Wir müssen bewusst darauf achten sauberen Code zu schreiben und einen Teil unserer Zeit dafür investieren. Wir müssen unseren Code immer wieder reflektieren, damit wir immer besser werden.
Kommentare