Tiefe Verschachtelung – A common Code Smell
Geschrieben am 10. Dez 2009 von Cem Derin
Ich habe es schon getwittert: Heute habe ich keinen Bock auf mein Brawler-Projekt. Stattdessen gibt es mal wieder einen kleinen Praxis-Tipp von mir. Dabei geht es um etwas, das sicher jeder von uns kennt: Unverhältnismäßig tiefe Verschachtelung von Code. If-Condition in If-Condition, unnötige Else-Statements, dick vermantelte Methodenaufrufe.
Für mich persönlich ein Code-Smell. Zumindest ekele ich mich, wenn ich soetwas sehe:
<?php
if(isset($_POST['username'])) {
if(trim($_POST['username']) != '') {
if(isset($_POST['password'])) {
if(trim($_POST['password']) != '') {
// … und so weiter
}
}
}
}
?>
Natürich ist das ein Extrembeispiel. In der Realität sind zumindest die Prüfungen auf ein Feld in einer If-Condition zusammengefasst. Trotzdem: Werden mehr als 2 übergebene Werte geprüft, wird das ganze zu einem unübersichtlichen Haufen Mist. Das mag nun für manche Hart ausgedrückt sein, aber es ist leider so. Und spätestens wenn man so etwas warten oder ergänzen muss, wird mir jeder zustimmen.
Trotzdem ist das nur die schlimmste Erscheinungsform des von mir gemeinten Problems. Eine andere sind zum Beispiel Wahnwitzige Switch-Case-Konstrukte, bei denen jeder Case dutzende bis hunderte Zeilen Code hat. Oder Funktionsaufrufe wie dieser hier:
<?php addtoquery($query, mysql_real_escape(myCustomCheckFunc(ucfirst(strtolower($value))))); ?>
Sicher, auch wieder ein eher merkwürdiges Szenario, aber ein definitv real auftauchendes Problem. Und keineswegs lediglich in ermangelung von OOP auftretend. Das hier habe ich auch schon gesehen:
<?php $table->select($table->statement()->where($user->username)->where($user->password()))->getBuddies()->count(); ?>
Ein Wahnsinniges Konstrukt und jeder außer der Autor wird sich fragen: Was zum Geier passiert hier?
Ich könnte noch viele weitere Beispiele bringen: Merkwürdig und in scheinbar sinnloser Reihe aufeinanderfolgende Zeichenkettenverknüpfungen und Funktionsaufrufe; Tief in Klammern gefasste und sich über mehrere Bildschirme erstreckende Berechnungen; und, und, und. Wer solche Dinge noch nicht gesehen hat, der ist entweder ein Meister seines Fachs oder lügt! Spätestens wenn man an Uralten Legacy-Code muss, wird einem soetwas üder den Weg laufen.
Aber ich beruhige mich ja schon wieder. Meckern kann jeder. Konstruktibe Kritik ist gefragt. Daher hier ein paar meiner persönlichen Richtlinien für möglichst gut lesbaren Code!
Keep it Short – 80-chars-short!
Als ich meine ersten Scriptzeilen schrieb, für die ich Geld bekommen habe, habe ich mir möglichst gute Software besorgen wollen. Ich hab mir vom damals besten Entwickler den ich kannte (und das ist er im übrigen Heute noch. Er weiß schon, dass er gemeint ist) Tipps geholt. Damals empfahl er mir Crimson Editor für Windows. Das Ding gibt‘s heute AFAIK gar nicht mehr. Trotzdem bin ich damit Ewigkeiten rumgehampelt. Jedenfalls: Dieser Editor hatte eine feine Linie im Textbereich. Und zwar genau nach dem 80. Zeichen. Damals hatte ich keine Ahnung, warum die da war. Wenn ich ehrlich bin: Ich hab es heute auch nicht. Trotzdem habe ich mich unterbewusst an dieser Linie orientiert, und Zeilen möglichst nicht über diese Linie hinausgeschrieben. Und nun dürft ihr einen Blick auf meinen derzeit geöffneten Editor werfen:
Ihr seht: Ich habe diese Linie immer noch. Und ich versuche immer noch, sie möglichst nicht zu überschreiten. Das sich das nicht immer machen lässt, ist klar. Vor allem wenn – wie man auch im Screenshot sieht – die Linie lediglich um eine Handvoll Zeichen überschritten wird, nehme ich mir die Freiheit heraus, das zu ignorieren. Trotzdem: Allein die Tatsache, dass ich eine räumliche (gibt es eigentlich das Wort „breitliche“?) Begrenzung habe (jetzt fällt mir das richtige Wort ein: horizontale
), bin ich gezwungen, öfter mal die Enter-Taste zu schlagen. Und das wiederum führt alleine schon dazu, dass ich keine Ewig langen ineinander Verschachtelten Calls mache.
Randanekdote: Ich habe mal in einem Tochterunternehmen eines großen deutschen Hosting-Anbieters gearbeitet. Man hat sich Gegenseitig öfter mal mit Code-Base ausgeholfen. Damals habe ich nur auf einem Monitor gearbeitet. Eines Tages brauchte ich eine gute Mailklasse (warum freie nicht gingen, weiß ich nicht mehr) und bekam eine vom Kollegen der Mutterfirma. Absolut unlesbar, die Person nutzte 10(!) Spaces als Tabs, hat zwischen allem eben solche plaziert ($this - > method ( $var ) ; – und das ist kein Witz!) und darüberhinaus auch noch Ellenlange Methoden- und Variablennamen benutzt. Warum er trotzdem damit arbeiten konnte war klar, als ich ihm einen Besuch abstattete: Sein Editor erstreckte sich über beide Monitore …
JSON! – hä?
Ja, wie komme ich nun auf JSON? Ganz einfach: Als ich JSON das erste mal bewusst wahr genommen habe, war ich fasziniert von der Schlanken Art und Weise, Informationen zu verpacken, dabei fast immer gut 30% weniger Overhead zu brauchen als mit XML und das ganze dabei noch für das menschliche Auge lesbar zu halten.
Mir fiel auch sehr schnell auf, dass man das im Grunde auch auf PHP-Code anwenden kann. Daher sehen Arrays bei mir nicht mehr so aus:
<?php
$a = array('name' => 'Cem', 'url' => 'http://phphacker.net/', 'likes'
=> array('beer', 'music', 'tattoos'), 'dislikes' => array('ignorance'
, 'spam', 'fish'));
?>
Sondern so:
<?php $a = array( 'name' => 'Cem', 'url' => 'http://phphacker.net/', 'likes' => array( 'beer', 'music', 'tattoos' ), 'dislikes' => array( 'ignorance', 'spam', 'fish' ) ); ?>
Wesentlich leserlicher, oder? Das Konzept lässt sich im Grunde auf alles Anwenden: Methoden und Funktionen mir vielen Parametern, Fluid-Interfaces, Zeichenkettenverknüpfungen. Und selbst wenn man mal anfängt wild zu verschachteln: Es ist zumindest noch ersichtlich, was passiert.
Ein Tab ist 4 Spaces lang und darf nur viermal hintereinander vorkommen
Ok, das was jetzt kommt, ist eine Regel, die ich meist eher halbherzig befolge. Das liegt daran, dass diese Gewohnheit im Gegensatz zu den anderen beiden aktives mitdenken erfordert – und das kommt mir, wenn ich ein kniffeliges Problem löse, eher ungelegen. Trotzdem will ich kurz erläutern, um was es geht.
Man Strafe mich nun Lügen, aber ich bin mir fast sicher, dass ich diese Regel (nicht in diesem Wortlaut) bei den offiziellen Coding-Guidelines des Linux-Kernel gelesen habe. Dort wird empfohlen, die Tab-Breite auf 4 Spaces einzustellen. DAS mache ich definitiv nicht – bei mir sind es nur 2. Und ich finde auch, das sollte jedem selbst überlassen sein (wenn man weiß, wie man „tabbt“, so dass es bei allen gleich ausschaut
). Interessant fand ich aber, dass man nach Möglichkeit nur maximal 4 Ebenen einrücken soll. Begründet wurde das mit besserer Lesbarkeit und nicht so schnell auftretenden Ermüdungserscheinungen.
Randanekdote: Dabei stand auch, dass man nicht mehr als 2 Stunden am Stückprogrammieren sollte, um keinen schlechten Code durch Müdigkeit zu erzeugen. Das lässt sich im Berufsleben ja leider eher schwer umsetzen. Allerdings rührt das sicherlich nicht auch zuletzt daher, dass man in der Regel nach Feierabend (und das bedeutet bei den meisten nach 8 Stunden und Mehr, die man schon programmiert hat) an Open Source Projekten arbeitet.
Lieber einmal mehr als einmal zu wenig zuweisen
Ich persönlich finde Parameter, die aus direkten Rückgaben von anderen Methoden oder Funktionen übergeben werden, immer irgendwie … nervig. Es ist nicht falsch (und pauschal kann man das sowieso nicht festlegen), aber oftmals erschwert es mir die Leserlichkeit. Falls ihr nicht nachvollziehen könnte, was ich meine:
<?php $this->transform($this->getValue($request->getName())); ?>
Hier könnte man durch oben erwähnte Einrückung schon einiges an Übersichtlichkeit herausholen, aber es tut auch nicht weh, wenn man es so macht:
<?php $value = $this->getValue($request->getName()); $this->transform($value); ?>
Man darf nie vergessen, das Code gewartet werden muss, damit er nicht verottet. Oder das Funktionalitäten ergänzt oder geändert werden müssen. Dann ist es hilfreich, wenn der Entwickler, der diese Stelle überarbeitet nicht erst den Call auseinandernehmen muss (evtl. auch nur, um ihn zu verstehen), um am Ende einzelne Werte nochmal zuweisen zu müssen, weil diese noch woanders hingeschickt werden – und dann dabei evtl. auch noch Gefahr läuft Namen zu vergeben, die schon vergeben wurde. Sicher, in der Regel sollte die IDE meckern, muss aber nicht sein. Und sicher, spätestens PHP wird meckern … muss aber auch nicht sein, wenn es eine Stelle ist, die beim schnellen hacken nicht geprüft und somit durchlaufen wird.
Skizzen nutzen
Wenn ich einen Komplexen Ablauf programmiere, erstelle ich die Methode inkl. Signatur und erstelle mit Kommentaren eine Beschreibung dessen, was ich gleich Programmieren werde. Beispiel:
<?php // determine class name // init instance of class // call requested method // return result ?>
Und ihr werdet nun auch alle wissen, dass man seinen Code kommentieren sollte, daher wird euch das nicht komisch vorkommen. Allerdings: Wenn es dann an konkrete Befehle geht, passiert bei mir folgendes:
<?php // determine class name $className = $this->determineClassName(); // init instance of class $instance = $this->getInstanceOfClass($instance); // call requested method $return = $this->callMethod($instance, $method); // return result return $return; ?>
Ich deligiere den Krempel in weitere Methoden. Und das mache ich in der Regel so lange, bis Methoden nicht länger als eine halbe Seite sind (also so um die 30-50 Zeilen). Das hat en angenehmen Nebeneffekt, dass man so wiederverwertbare Methoden baut, auch wenn man jetzt noch gar nicht weiß, dass man genau das Stück Code bald noch einmal braucht.
So, ich hoffe ich konnte euch ein paar nützliche Sachen mit auf den Weg geben. Feedback ist wie immer willkommen!

#001
Sehr guter Artikel, wirklich. Das meiste setze ich so ein – und bei Kollegen auch durch, wenn es Coding-Guidelines geht, die wir in der Firma einsetzen.
Ein Kommentar zu dem Kapitel “Lieber einmal mehr als einmal zu wenig zuweisen”: Bei Google findet man sowas schlecht, solange man die Rückgabe nicht noch für anderes nutzt. Steht in den “Google Page Speed” Coding Guides zu PHP.
Da kann aber jeder getrennter Meinung sein, beim debuggen ist es zumindest hilfreich zu wissen, was welche Funktion (warum) zurück gibt.