Heute ist der 3.11. und laut dem php hacker ist heute der „Developers Shame Day„. Heute will ich daher einen kleinen Codeabschnitt mit kurzer Analyse zeigen, der nicht gerade der Hit ist.
Der folgende Code ist ca 7 Jahre alt. Ich hätte gerne einen noch älteren Code hervorgeholt, leider konnte ich keine Backups des entsprechenden Projekts finden. Der Code wäre dann ca 11 Jahre alt.
function isSpecialRight() {
$query = "select special from benutzer where id='" . $this->user_id . "'";
$erg = mysql_query($query, $this->DBLink);
$out = false;
while ($item = mysql_fetch_array($erg)) {
$out = $item['special'];
}
return $out;
}
negativ:
- Die Methode hat keinen Kommentar. Da der die Benamung auch nicht besonders ist, kann ich nicht mehr sagen, welches besondere Recht hier geprüft wird.
- Weiter hat die Methode keinen Modifier. Man könnte sagen, dass die Methode eben
publicist. Aus dem Zusammenhang heraus wäreprivateaber eher angebracht. - Sehr ungeschickt ist auch die
While-Schleife zu benutzen. Die user_id sollte ein Primärschlüssel sein und von daher sowieso nur einmal existieren. - Die Datenbank-Kommunikation würde ich heute entweder per PDO oder mit einem ORM-System (wie bspw Doctrine) realisieren.
positiv:
- Die Verbindung zur Datenbank wird aus einer Klassen-Variable bezogen. Und somit hat man eine Art Dependency Injection für Arme. Bei einem Unit Test könnte man diese Variable mit einem Mock überschreiben.
- Der Rückgabe Wert wird sinnvoll initialisiert. Immerhin das passt schon mal 🙂
Sicher gibt es noch einige Kommentare und ich hoffe nicht schon alle Punkte vorweg genommen zu haben. Insgesamt finde ich den „developers shame day“ eine interessante Sache.