// der php hacker

// archiv

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:

Bild 3

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!

Geschrieben in Entwicklung 10 Kommentare

#001
10. Dez 2009

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.


#002
10. Dez 2009
uli

@sascha
Die “Google Page Speed” Coding Guides zu PHP wurden schon zu Hauf auf diversen Blogs zerrissen und vieles davon stimmt einfach nicht, was dort steht. Darauf würde ich also nicht bauen

@cem
Bei deinem allerletzten Snippet zu den skizzen sind die Kommentare eher störend als hilfreich, nachdem du die Aufgaben schon schön brav in methoden gepackt hast. Nachdem die Kommentare ähnlich oder auch genauso heißen wie die Methoden, würde ich an dieser stelle auf die Kommentare verzichten.


#003
10. Dez 2009

Also sowas:
// return result
return $return;

würde ich ja auf keinen Fall so stehen lassen. Einzelne Code-Zeilen in Methoden sollten selbsterklärend sein, finde ich.

Und auch Methoden sollten so sparsam wie möglich kommentiert werden, also nur soviel wie zum Verständnis derselben nötig ist. Wer kennt nicht die berühmten getX()-Methoden, bei denen steht „Returns X“ und die einzige Zeile Code innen drin ist return $this->X?


#004
10. Dez 2009
Sebastian

@ Timo: sehe ich nicht so. Wenn man nämlich am Ende eine Dokumentation generieren will, ist es trotzdem gut, wenn man zu jeder Methode was geschrieben hat, dann bleibt einem auch jeder Block in den Source erspart.


#005
10. Dez 2009

Gute Idee, mit dem Skizzieren von Funktionen mit Kommentaren…

Eine kleine Anmerkung: Die 80 Zeichen pro Zeile stammen aus den Konsolen-Zeiten (für für manche ja immer noch aktuell sind ;) ). Dort hatte eine Standardkonsole immer 80 Zeichen…nur der Vollständigkeit halber ^^


#006
10. Dez 2009
Cem Derin

@uli & timo: Ihr habt beide Recht, die Kommentare sollten “eigentlich” weg. Ich habe sie dort der Vollständigkeit halber stehen gelassen. Im Nachhinein in der Tat verwirrend, aber ich hoffe, dass die Leser die Kommentare auch lesen. In der Praxis verschwindet bei mir auch jeder Kommentar, der selbsterklärendes kommentiert.

@dubbel: Danke für die Info. Man lernt nie aus – und so habe ich mir das Googlen gespart ;)

@Sebastian: Sehe ich ähnlich. Ich differenziere allerdings zwischen Kommentaren und Doc-Blöcken. Und selbst wenn dort das selbe in “menschlicher” Sprache steht: Weh tuts keinem und Brot frissts auch keins.


#007
12. Dez 2009

Gute Ideen – vor allem die Array-formatierung, genauso muss das aussehen. Allerdings: 80 Zeichen sind echt retro, ich würde ‘mal so 120-140 empfehlen – Monitore sind in den letzten Jahren eher breiter geworden :)

Die Sache mit den Tabs ist extreme Geschmackssache – genau wie Einrückung und Spacing, ich kann zum Beispiel das fehlende Leerzeichen nach dem if überhaupt nicht ertragen..
Das mit der Verschachtelung sehe ich auch so – aber ich bin gerade bei Obj-C und da geht teilweise unter 5 Klammern pro Zeile gar nix. Hängt echt von der Sprache ab.

Übrigens: “Code Complete 2″ solltest du mal lesen, falls noch nicht getan – da steht prinzipiell genau sowas drin (und noch viel ausführlicher und mehr davon).


#008
12. Dez 2009

Aber das die Monitore breiter sind ändert ja nichts daran, dass lange Zeilen Code schwerer nachzuvollziehen sind, als kurze.

Falls du wegen Kommentaren meinst, dann ist das zwar schon richtig aber meine IDE kann zum Beispiel nur eine Linie und in dem Fall bräuchtest du ja zwei, eine für deine Code Zeilen, einer für die Kommentare ;-)

Gruß Seb


#009
13. Dez 2009

Ich meinte mit das der Monitorbreite: Man sollte sich nicht sklavisch an irgendwelche Regeln aus der DOS-Ära halten und daher kann man dann auch mit Tabs (oder Spaces :) , strukturellen Einrückungen, aussagekräftigen Funktionsnamen etc. entspannter umgehen, als wenn man nur 79 Zeichen zur Verfügung hat.


#010
17. Dez 2009
Cem Derin

Prinzipiell gebe ich dir was die zeichenbeschränkung pro Zeile angeht recht. Aber wirklich oft kommt es bei mir ehrlich gesagt nicht vor, dass eine Zeile tatsächlich deutlich länger sein muss. Und wenn es trotzdem leserlich ist – mache ich auch Ausnahmen ;)

// kommentieren

// senden
theme von mir, software von wordpress, grid von 960 grid system. funktioniert in allen browsern, aber der safari bekommt das mit der schrift am schönsten hin.