Archiv der Kategorie: refactoring

Method-Chaining in eigenen PHP-Projekten einsetzen

Klassen und Objekte kennt ja nun mittlerweile fast jeder, ein PHP-Framework wie ZendFramework oder CodeIgniter haben sich sicherlich auch schon viele zumindest mal angesehen … will ich hoffen. Nun, auch wenn man die Frameworks nicht direkt einsetzt, so kann man doch viel von ihnen lernen. Viele der Techniken, die in bekannten PHP-Frameworks eingesetzt werden, entstanden entweder aus dem großen Ästhetischen Faible, den nun mar jeder Entwickler hat oder schlicht aus Faulheit der Programmierer 😉

Egal, eines der – wie ich finde – tollen Features in vielen Frameworks ist die Möglichkeit, mehrere Funktionen eines Objektes hintereinander aufrufen zu können, ja, es sieht schon fast wie ein normaler Satz aus, was dann im Editor steht und seinen Dienst verrichtet. Diese Möglichkeit der sog. “Fluent Interfaces” nennt man “Method-Chaining” (und wieder im Bullshot-Bingo gewonnen, *strike*).

Kleines Beispiel, kennen wir alle, unsere Basisklasse:

class Base1 {
function macheEins() {
// some magic here
}
function macheZwei($parameter) {
// more magic here
}
}

Das ganze rufe ich nun auf:

$myBaseClass = new Base1();
$result1 = $myBaseClass->macheEins();
$result2 = $myBaseClass->macheZwei($result1);

Was passiert? Das Ergebnis des Aufrufs von “macheEins” ist der Parameter für “macheZwei”. Das ganze sieht strukturiert aus und ist lesbar, was also soll man besser machen können? Nein! Bitte jetzt nicht sagen, man könne doch sowas machen:

$myBaseClass = new Base1();
$result2 = $myBaseClass->macheZwei($myBaseClass->macheEins());

Sicher, es funktioniert, aber: NEIN! Macht das nicht! Warum? Ganz einfach: Debugge das mal, viel Spaß. Denn wenn man erst einmal mit so einem falschen Verhalten anfängt, dann verschachteln sich schnell auch mal 5 oder 8 Funktionen ineinander und finde dann mal den Fehler, viel Spaß! Also: Ganz klares “So nicht!”.

Aber wie dann?

Zunächst müssen uns im klaren sein, was die Klasse macht. Offenbar braucht “macheZwei” ein Ergebnis einer Berechnung einer anderen Funktion der Klasse. Dieses Ergebnis könnte man doch genauso innerhalb der Klasse speichern und dann benutzen.

class Base2 {
private $valueHolder;
function macheEins() {
$this->valueHolder = someMagic;
return $this->valueHolder;
}
function macheZwei() {
return $this->valueHolder * someMoreMagic;
}
}

Schon erfüllt die Klasse auch die Anforderungen, aber … so richtig “fluent” will das ganze nicht werden, obwohl ja nun im Aufruf von “macheZwei” der Übergabeparameter fehlt. Was nun? Und wie sieht denn so ein “Fluent Interface” mit “Method-Chaining” nun aus? Der Aufruf sähe in unserem Beispiel in etwa so aus:

$myBaseClass = new BaseFluent();
$result2 = $myBaseClass->macheEins()->macheZwei();

Aber unsere derzeitige Klasse unterstützt das nicht! Wie bekommen wir unsere Klasse nun “Fluent”?

Nun, dazu müssen wir diese massiv umbauen. Der größte Umbau ist, dass die einzelnen Methoden nicht mehr direkt die Ergebnisse liefern, sondern “nur” das Objekt selbst zurückgeben … und darin liegt auch schon der ganze Trick. Den Methodenaufruf kann ich immer nur auf einem Objekt machen. Eine Methode, die mir einen Basisdatentypen zurück liefert, kann ich dafür nicht gebrauchen, da ich auf diesem Basistypen (int, String, array, …) keine weiteren Methoden meiner Klasse aufrufen kann.

Nehmen wir mal an, im letzten Code stünde statt “new BaseFluent” ein “new Base2”. Dann würde der Aufruf:

$result2 = $myBaseClass->macheEins()->macheZwei();

folgendes bedeuten: Rufe die Methode “macheEins” auf dem Objekt “myBaseClass” auf, diese gibt den Datentyp von valueHolder zurück (nehmen wir mal an, es wäre ein integer mit dem rein zufälligen Wert 42), rufe dann auf dem Objekt 42 die Methode “macheZwei” auf … *meep* Fehlermeldung, “42” ist kein Objekt, hat daher generell keine Methoden und erst recht keine spezielle Methode mit dem Namen “macheZwei” also Fehler und Script Abbruch.

Der Trick besteht nun darin, dass die Methoden des Objektes nicht mehr die eigentlichen Ergebnisse zurückgeben, sondern das Objekt der Klasse selbst; darauf darf man ja dann auch wieder Methoden derselben Klasse aufrufen, also bauen wir flugs die Klasse um:

class BaseFluent {
private $valueHolder;
function macheEins() {
$this->valueHolder = someMagic;
return $this;
}
function macheZwei() {
$this->valueHolder *= someMoreMagic;
return $this;
}
function getValueHolder() {
return $this->valueHolder;
}
}

Man erkennt nun, wohin die Reise geht. Getter und Setter werden implementiert, um die Daten zu holen, die Methoden geben uns $this zurück, worauf wir weiterhin Klassenmethoden aufrufen können und wir haben nun unser “Fluent Interface” für “Method-Chaining” in PHP realisiert; war doch gar nicht schwer und hat auch gar nicht weh getan, oder?

In der freien Wildbahn trefft ihr auf diese Art des Codens übrigens ganz stark beim ZendFramework an, bei CodeIgniter geht es wohl auch, denke ich (ich mache mich da erst seit kurzem fit und bitte alle CI-Fans, meine Unwissenheit zu entschuldigen). Es macht aber auch Spaß, dass bei eigenen Klassen umzusetzen, die sowieso umgearbeitet werden sollen. Sieht einfach viel übersichtlicher aus. Und noch ein Tipp: Mehr als zwei Verkettungen sollten untereinander stehen, also so:


$myBaseClass->macheEins()
->macheZwei()
->macheDrei()
->undNochMehr();

Viel Erfolg damit …

KISS – oder wie umständlich muss es noch werden?

Passend zum DRY-Artikel nun der zum Thema KISS. Nein, nicht die Rock-Band-KISS, sondern ein Prinzip der Softwareentwicklung, das besagt: „Mache es so einfach wie möglich!“.

Immer wieder sehe ich Code, der akademisch bestimmt mit Note 1 ausgezeichnet wird. In Java, C++, Delphi oder anderen compilierenden Sprachen hat das ganze auch durch seinen Sinn und seine Daseinsberechtigung. Aber in PHP sieht die Sache anders aus.

PHP ist eine Interpretersprache und wird es auf absehbare Zeit auch erstmal bleiben. Interpretersprachen haben im Grundprinzip eines gemeinsam: Sie durchlaufen den Code während der Ausführung Zeile für Zeile, Schritt für Schritt. Und genau da liegt der Schwachpunkt: Je mehr Zeilen ein Code hat, desto länger braucht der Interpreter, den Code zu durchlaufen. Je länger der Interpreter braucht, umso größer wird die „Time-to-Response„, also diejenige Zeit, die zwischen Anforderung an den Server (Aufruf der Seite) und Antwort des Servers (Auslieferung der Seite, also des generierten Quelltextes) liegt. Und diese Zeit sollte „as small as possible“ sein (wer wartet schon gern auf seine Seite).

Dieses Problem potenziert sich meist, da der normale User erstmal eine (allgemeine) Startseite ansurft und sich dann durch die Seite bewegt; dabei ruft er meist spezialisierte Seiten mit mehr Aufgaben für den Server auf. Mehr Aufgaben, mehr Inhalt, mehr Code, länger warten, nicht gut!

Warum nun das ganze hier im Zusammenhang mit PHP?
In PHP lassen sich selbstverständlich schöne Objektorientierte Konstrukte bilden, die auch von vielen Seiten als „richtig“ proklamiert werden. Ein Beipiel sei dies (in Pseudocode, nicht wundern):



holeBenutzerdaten();
// mach was damit


// ab hier nun die funktionen, diese liegen alle
// in unterschiedlichen klassen, dateien usw.

function holeBenutzerdaten()
{
prüfeEingabe();
ladeBenutzerdaten();
return;
}

function prüfeEingabe()
{
prüfeSpezifischeEingabe;
ladeIrgendwasUnwichtiges;
prüfeNochmal;
}

function prüfeSpezifischeEingabe()
{
prüfeObEingabeValideIst
ja: prüfeDieEingabeNochmalMitWasAnderem
nein: ladeFehlerbehandlung; gibFehlerAnFehlersystem; zeigeFehler;
}

// So, und nun noch 20 Dateien mehr und ca. 60 Funktionsaufrufe

Nicht schön, denn hier wird, anstatt das Problem zu erledigen, von einer Funktion zur nächsten gesprungen. Unschön dabei: Jede Zeile kostet Zeit und die gilt es in einem guten Projekt so gering wie nur möglich zu halten.

Es gilt das Prinzip: So viel wie nötig, so wenig wie möglich! Nicht mehr, nicht weniger. Dazu aber noch später mehr.

Und nun Beispiel 2: Die reine Lehre sagt, dass veränderliche Werte in Variablen abgelegt werden sollen und diese Variablen dann an einen einheitlichen Ort. Nennen wir das mal „Config-Dateien“, dann kann sich jeder was drunter vorstellen. Diese Config-Files kennen wir alle, meist stehen da mindestens die MySQL Zugangsdaten drin, manchmal auch noch was anderes, zumeist belangloses. Das ist soweit auch gut und richtig und sollte – in einem gewissen Rahmen – auch gemacht werden … allerdings sollte man es dabei nicht übertreiben.

In diesem Beispiel möchte ich zeigen, wie man – unter dem Aspekt des Konfigurierens und Auslagerns – eine ganz simple Sache sehr kompliziert machen kann … und dafür in gewissen Kreisen sicherlich noch großen Beifall ernten wird.

Urspung des ganzen ist ein Aufruf, ähnlich wie dieser:


mysql_query("SELECT * FROM user");

Warum schreibt nun in die Config-Dateien nicht rein, wie die Tabellen heißen und benutzt dann nicht den statischen Wert, sondern die Variable? Und warum nehmen wir dann nicht eine Funktion? Die kann man dann immerhin noch mit Unit Tests auf ihre korrekte Funktion testen!? Das sieht dann so aus:


public function getTbluser() { return 'meineUserTabelleInMysql'; }

und die kann man dann so nutzen:


mysql_query("SELECT * FROM ".getTbluser());

Toll, oder? Ändert sich nun der Tabellenname, dann muss man das nur an einer Stelle ändern und gut ist. Kommt doch dem DRY-Prinzip zugute.
Noch besser wäre es doch, wenn wir nun auch alle Feldnamen in festlegen … weil … die können sich ja auch ändern. Also los, Feldnamen auch. *tipptipptipp* Toll. Ach ja, die Variablennamen könnten sich ja mal ändern. Also auch die rein und per $$ eingebungen, damit es auch geht.

Und der Code sieht dann doch gleich viel … übersichtlicher aus:


mysql_query("SELECT ".getTblUserField1().",".getTblUserField2()." FROM ".getTblUser()." WHERE ".getTblUserFieldSort()." = $$configFileTblUserFeldSort");

Super, oder? Kommt man nun noch dazu, diese ganzen, sich ständig verändernden SQL Kommandos noch zu ersetzen, dann hat man doch ein völlig einfach konfigurierbares System vor sich, oder etwa nicht?

Ihr merkt es schon, ich drifte ins Zynische ab – aber nur gaaanz leicht, das mag daran liegen, dass ich von diesem Code in letzter Zeit zu viel gesehen habe – , nochmal also zur Klarstellung:
Nein, so macht man es nicht!

Warum nicht? Dazu gibt es gleich mehrere Punkte.
Zum einen kann man es nicht mehr lesen. Das ist ein sehr wichtiger Punkt, unterschätzt das nicht, denn zum einen sitzt ihr nicht bis in alle Ewigkeit an einem Code – nichts ist schlimmer, als nach Monaten wieder alten Code lesen zu müssen und nicht zu verstehen – und zum anderen werdet ihr an der Qualität eures Codes auch innerhalb der Entwicklungsabteilung gemessen! Schwer verständlichen Code zu schreiben mag einen Marketing-Menschen noch leicht beeindrucken können, ein guter Programmierer dagegen wird euch – wenn ihr Glück habt nur leise – verfluchen.

Zum anderen muss der Interpreter jedesmal wieder in die entsprechende Funktion springen und das braucht Zeit. Nein, viel ist das nicht, wenn das nur eine Tabelle ist, nur ganz, ganz wenig Zeit, wirklich. Allerdings: Hat man erst so eine schöne Funktion, die einem die entsprechenden Tabellen und Felder referenziert, dann benutzt man das doch nicht nur für die Abfrage selbst, nein, auch im Code kommen dann anstelle der Feldnamen die Funktionsnamen vor.


$res = mysql_query("SELECT ".getTblUserField1().",".getTblUserField2()." FROM ".getTblUser()." WHERE ".getTblUserFieldSort()." = $$configFileTblUserFeldSort");
$name = $res[getTblUserField1()];
// anstelle von
// $name = $res['name'];

(ja, ich weiß, dass da ein mysql_fetch_irgendwas fehlt)

Und das für jede Tabellenabfrage im Code … und jedes Feld … in jeder Abfrage … und dazu noch in jedem Kontrollkonstukt im Code, also jeder for-Schleife, jeder Ausgabe, in jedem if, in jedem switch und und und … Leute, das summiert sich. Da kommen schnell dutzende Funktionssprünge vor und das sind Zeiten, die nicht sein müssen!

Warum sollte meine Anwendung langsamer sein, als es sein müsste. Die wird von ganz allein langsamer, sobald nämlich immer mehr Benutzer gleichzeitig was wollen, dann summieren sich die Zeiten nicht nur, nein, die multiplizieren sich mit jedem Request! Also, jeder Flaschenhals ist es wert, dass er refaktorisiert wird. Warum nicht gleich von Anfang an alles richtig machen?

Und mal ehrlich: Wie oft ändern sich die Tabellennamen und man ihr als Entwickler müsst wirklich nur die Configs ändern? Na, ehrlich, komm *inDieSeiteStubs* Na also, noch nie gesehen, jede Umstellung der Tabellennamen hatte schon immer eine mehrstündige Umstellungsphase begründet … zusammen mit dem Verlust einiger wichtiger Gramme Körpergewicht 😉

Und hatten wir nicht eben erst erwähnt, wie wichtig die Lesbarkeit des Codes ist? Sicher, ihr habt das Projekt grade erst begonnen und wisst noch alles aus dem Kopf. Aber, wie gesagt, ihr seit meist nicht allein an einem Projekt und – selbst wenn – auch nicht immer „am Stück“ dabei. Euer Kollege würde gern wissen, warum ein bestimmter Request langsam ist oder gar nicht funktioniert und schaut im Code noch und trifft auf so einen Query. Nun muss erstmal mühsam der eigentliche, der „echte“ Query zusammengebaut werden, denn der Kollege hat keine Ahnung, welche Felder ihr ansprecht.

Aber der Tabellenname könnte sich doch mal ändern!
Warum sollte sich der Tabellenname ändern? Ja, auf Shared-Hosting-System kommt es schon mal vor, dass man in einen Webspace viele Webanwendungen packt und da kann es auch vorkommen, dass mehrere Anwendungen ihre Benutzertabelle eben ‚user‘ nennen. Warum auch nicht, es ist der passendste Name dafür! In solchen Fällen – wenn ich also weiß, dass so eine Situation eintreten kann – benutze ich die Option eines Praefix, der jedem Tabellennamen vorgestellt wird und an zentraler Stelle definiert ist.


// Im Config-File
define('DB_PRAEFIX','myPraefix_');

// Im Code
mysql_query("SELECT * FROM ".DB_PRAEFIX."user");

Das hat den Vorteil, dass man den SQL immer noch lesen kann und auch die Tabelle auf dem DB-Server wiederfindet, auch wenn man keine Kenntnis über den Inhalt von DB_PRAEFIX hat. Der Code bleibt lesbar und so ziemlich jeder Entwickler weiß auch, warum ihr das macht.

DAS ist es zum Beispiel, was dass KISS Prinzip sagt: Macht es einfach! Nicht nur euch, sondern auch anderen. Nein, ihr sollt nun dabei nicht vergessen, Sicherheitsmechanismen einzubauen oder Usereingaben zu filtern, wir wollen auch nicht zurück ins PHP3 Monolithen-Zeitlalter, aber dazu bedarf es keiner Funktion, die eine Funktion aufruft, die eine Funktion aufruft, die wiederum … ihr ahnt es schon.

Meine KISS Anforderungen sind ungefähr diese

  • – Die Gesamttiefe der Funktionsaufrufe darf 3 Ebenen nicht überschreiten (die aufgerufene Seite nicht eingeschlossen). Je weniger, desto besser.
  • – Funktionen sollten so wenig wie nötig aufgerufen werden
  • – Funktionen sollten etwas machen, nicht nur Namen liefern (return ‚tbluser‘; )
  • – Funktionen sollten das machen, wofür sie da sind. Nicht mehr!

„Klar“, sagt nun jeder, „aber wie bekomme ich dies oder das dann hin, das geht dann gar nicht mehr“.
Bei vielen dieser Fragen kann man sagen, es ist die berühmte Ausnahme von der Regel und sicherlich ist hier und da eine Ausnahme sicherlich ratsam. Man soll schließlich das ganze gerede und die ganzen schönen Buzzwords nicht als „In-Stein-gemeisselt“ verstehen. Leider zeigen viele dieser Ausrufe aber auch, wie groß die Abhängigkeit von bestimmten Frameworks oder Programmierparadigma ist.

Ich will hier keine Diskussion auslösen über Sinn und Unsinn von Frameworks oder Paradigmen, ich zeige – wie so oft in der Softwarearchitektur – das Optimum. Der Rest liegt bei euch, der Weg ist das Ziel 😉

Über eure konstruktive Meinung dazu würde ich sehr freuen – obwohl ich mehr mit empörten Kommentaren rechne *schnellUnterDenTischDuck*!

DRY – redundante Funktionen in PHP vermeiden

DRY – Don’t repeat yourself.
Das kennt jeder, das _sollte_ jeder wissen. Und so gut wie jeder, der etwas Ahnung hat, versucht es so oft es geht umzusetzen.

Allerdings: Das gilt nicht nur für Funktionsnamen, das DRY-Prinzip geht weiter. Es meint IMHO auch, dass man für viele ähnliche Abfragen sich am besten Wrapper baut, die dann die Anfrage an eine Funktion leiten, die die eigentliche Antwort produziert. Die Wrapper „füttern“ die eigentliche Funktion nur mit den richtigen Fragen.

Ein Beispiel: Siggi Scriptkid hat ein Klasse zur Benutzerverwaltung geschrieben, die braucht er in seinem aktuellen Projekt. Er speichert seine Benutzerdaten in einer MySQL Datenbank und muss nun seiner Klasse folgende Fragen stellen und erwartet folgende Antworten:

  • Hole mir den Benutzer mit der ID 5
  • Hole mir den Benutzer mit der Mailadresse xyz
  • (noch viele weitere, ähnliche Fragen mehr)

Siggi schreibt also ganz fleißig Funktionen in seine Klasse:
(Stark vereinfacht, es fehlen Filtern der Usereingaben, maskieren der SQL String und vieles mehr; der Code dient nur als Beispiel!)


class User
{
public function getUserByID($userid)
{
$ressource = mysql_query('SELECT vorname, name
FROM user
WHERE userid = '.$userid);
$user = mysql_fetch_assoc($ressource);
return $user;
}

public function getUserByMailadress($email)
{
$ressource = mysql_query('SELECT ID, mail, userlevel
FROM user
WHERE mail = "'.$email.'"');
$user = mysql_fetch_assoc($ressource);
return $user;
}
}

Nun trifft man auf folgendes Problem: Die Felder der jeweiligen Rückgabe sind unterschiedlich. Auch wenn Siggi nun hingeht, und in beiden Funktionen die Felder gleich setzt, ergeben sich Wartungsschwierigkeiten:

  • Was ist, wenn noch mehr, ähnliche Funktionen hinzukommen? Dann bekommt Siggi wirklich Arbeit!
  • Was ist, wenn Siggi der „user“ Tabelle noch mehr Felder hinzufügen muss und es 6 Funktionen gibt?
  • Was ist, wenn Siggi auch nur eine der Funktionen vergisst?

Um es kurz zu machen: Hier greift das DRY-Prinzip NICHT! Der o.a. Code sollte einem Entwickler, der nicht Siggi Scriptkid heißen möchte, nicht passieren.

Aber wie wende ich das DRY-Prinzip nun an?
Okay, ich schreibe mal meinen Vorschlag dazu:


class User
{
public function getUserByID($userid)
{
return $this->getUser('ID='.$userid);
}

public function getUserByMailadress($email)
{
return $this->getUser('mail='.$email);
}

private function getUser($where)
{
$ressource = mysql_query('SELECT ID, vorname, name, mail, userlevel
FROM user
WHERE '.$where.'
LIMIT 1);
$user = mysql_fetch_assoc($ressource);
return $user;
}
}

Ich schaffe eine private Methode, die die Daten aus der Datenbank holt. Diese ist die einzige Methode, die das macht, somit habe ich nur eine Stelle, in der ich zukünftig Felder ändern muss, wenn sich welche ändern.

Die „alten“ Funktionen verbleiben als Wrapper-Funktionen und übergeben der privaten Funktion nur die richtigen Parameter, um ihre eigene Aufgabe erfüllen zu können. Dann leiten Sie Antwort einfach als eigene wieder zurück.

Auf diesem Wege verinfache ich die Pflege, bin in der Lage, meine alten Funktionen weiterhin nutzen zu können – mit dem Vorteil, dass nun alle Antworten einheitlich sind – und bin darüberhinaus in der Lage, sehr schnell verschiedene neue Funktionen zu schreiben, die mit neuen Fragestellungen klarkommen, z.B. Benutzer mit E-Mail x und Passwort y.

Was haltet ihr von dieser Vorgehensweise. Ich selbst halte sie für praktiabel und gut, was meint ihr? Gut? Schlecht? Gibt es bessere Vorgehensweisen?

Benutzereingaben prüfen: Leere Strings

Ich kenne viele Wege, einen String daraufhin zu prüfen, ob der User

a) überhaupt was eingegeben hat und
b) ob der User außer Leerzeichen was eingegeben hat.

In jeder Sprache gibt es dafür viele Wege, einige gut, viele nicht so gut.

Heute mal ein Beispiel aus der Reihe “not so good”:

if

(strlen($tls_description) > 1 && substr_count($tls_description,“ „) >= (strlen($tls_description) – substr_count($tls_description,“ „)))
{
   $err = true;
}

Mein Ansatz, genauso gut, dafür lesbar und ebenso “effektiv”:

if

(strlen(trim($tls_description))<1)
{
   $err = true;
}

Nicht nur schneller, sondern auch lesbarer und nicht so WTF?

Art of Code, Refactoring, Sinn und Unsinn von Funktionen

Gegeben – aus welchem Grund auch immer – sei folgender Code:

function getRights(){
  $arr_rights = array();
  array_push($arr_rights, ‚admin‘);
  array_push($arr_rights, ‚client‘);
  array_push($arr_rights, ’superadmin‘);
  return $arr_rights;
}
function getRights_Superadmin(){
  $zws_rights = array();
  $zws_rights = getRights();
  return $zws_rights[2];
}
function getRights_Admin(){
  $zws_rights = array();
  $zws_rights = getRights();
  return $zws_rights[0];
}
function getRights_Client(){
  $zws_rights = array();
  $zws_rights = getRights();
  return $zws_rights[1];
}

Die Aufrufe der Funktionen getRights_Client(), …Admin() und …Superadmin() kommen im gesamten Script 16 mal vor, insgesamte Laufzeit der Datei, in denen diese Funktionen sind, liegt bei 0,113ms.

1

Folgendes Problem ergibt sich:
Bei jedem Aufruf einer der 3 Funktionen wird jedes Mal die Funktion getRights() aufgerufen, ergo wird diese Funktion 16 mal gerufen und verbraucht damit eine Laufzeit von 0.046ms, allein durch diese 16 Aufrufe.
An dieser Stelle – mal abgesehen davon dass es einfach “bad style” und ziemlich unoptimiert ist – sollte man refactorisieren.

Zuerst muss man den Sinn und Zweck verstehen, also: Was machen die Funktionen und warum?

Was machen die Funktionen?
Die Aufrufe der 3 Funktionen geben den Namen des jeweiligen aktuellen Benutzerrechtes wieder, der Name spiegelt sich in einer Datenbank wieder (ja, auch das hätte man anders machen können und sollen, aber dafür bleibt bei einer Bearbeitung von bestehendem Code nicht soviel Zeit) und diese Namen müssen auch 1:1 übernommen werden, will Mensch nicht den ganzen Code ändern (wozu Mensch normalerweise keine Zeit hat).

Welchen Sinn hat das ganze?
Die Funktion wie Sie oben stehen haben schon einen – wie ich meine – akademischen Sinn. Ziel des ganzen war es bestimmt, die jeweiligen Rechte einmal festzulegen und diese dann, mithilfe der Funktionen, an vielen Stellen abzufragen und zu vergleichen.

Der Weg über Funktionen und immer wieder kehrende Aufrufe anderer Funktionen ist jedoch im besten Falle als unpassend zu bezeichnen. Sicher: Es funktioniert, allerdings sollte ein guter Entwickler auch immer versuchen, seinen Code nicht nur funktional zu tippen, sondern auch “gut” und effizient.
Und effizient ist dieser Code nicht.

Kommen wir also zur Verbesserung: Wir refactorisieren den Code!

Aus Funktion und Sinn kann man leicht ersehen, dass das Mittel der Wahl am besten mit Konstanten umgesetzt werden kann.

Dazu definieren in der Datei im Kopfbereich diese Konstanten:

define(‚RIGHT_CLIENT‘, ‚client‘);
define(‚RIGHT_ADMIN‘, ‚admin‘);
define(‚RIGHT_SUPERADMIN‘, ’superadmin‘);

Nun ändern wir zuerst die Funktion getRights() entsprechend ab:


function getRights() {
  $arr_rights = array();
  array_push($arr_rights,RIGHT_ADMIN);
  array_push($arr_rights,RIGHT_CLIENT);
  array_push($arr_rights,RIGHT_SUPERADMIN);
  return $arr_rights;
}

Und nun können wir in den einzelnen Funktionen die Konstanten ins Spiel bringen.
Dadurch entfallen natürlich auch die Aufrufe von getRights() innerhalb der Funktionen, was eine Entlastung der Aufrufe von getRights() von 8 Aufrufen erbringt:

function getRights_Superadmin() {
  return RIGHT_SUPERADMIN;
}
function getRights_Admin() {
  return RIGHT_ADMIN;
}
function getRights_Client() {
  return RIGHT_CLIENT;
}
Und nun wird wieder profiliert und es ergibt sich folgendes Bild:
2

Nun wird die Funktion getRights() gar nicht mehr aufgerufen (der Aufruf erfolgt aber an anderen Stellen noch, die Ausführzeit nur dieser einen Datei liegt nun bei 0,054ms, also knapp 50% unter der eigentlichen Zeit und die 3 Funktionen haben ebenfalls verkürzte Ausführzeiten.

Die Funktionen selbst sind erhalten geblieben, der “Sinn” auch, denn die Rechte können zentral verwaltet und geändert werden.

Der nächste Schritt wäre, im gesamten Script die Aufrufe von getRight_Client(), …Admin() und Superadmin() gegen die entsprechenden Konstanten zu tauschen, so dass kein Aufruf mehr an diese Datei gehen würde. Dies kann man machen, ohne dabei den “Sinn” oder Funktion zu beeinträchtigen, allerdings fällt das wieder unten den Punkt “Zeit, die einem keiner bezahlt”, aber sinnvoll aus Sicht der Refactorisierung und des “Good styled code” wäre es sicherlich.

Noch am Rande: Alle Profilierungen wurden mit dem Zend Studio 7 vorgenommen, die Screenshots entstammen den jeweiligen Reports der “Execution Statistic”.

Kommentare dazu sind gern gesehen 😉

Refactoring – Alter Code im neuen Kleid … nicht nur für PHP

Grade in einem Code gefunden, der Aufruf der Funktion ‘getSession()’. Und nun mal die Preisfrage an alle PHP-Pro’s: Wieviel Code davon muss wirklich sein?
Aber erstmal der Originalcode:

function getSession(){
  $akt_timestamp = getTimestamp();
  $ses_timestamp = $_SESSION[getSession_Timeout()];
  if($akt_timestamp > $ses_timestamp){
    return false;
  }
else{
    if($akt_timestamp < $ses_timestamp){
      return true;
    }
  }
}
function getTimestamp() {
  return time();
}
function getSession_Timeout(){ return „usrTimeout“; }

*wow* Das sind 3 Funktionen, die ineinandergreifen, was?
Und nun – nochmal – die Preisfrage: Wieviel Code davon muss sein?
Ich behaupte: Nicht so viel, wie oben steht.
Beweis? Ich refaktorisiere den Code oder neudenglisch: Ich mache refactoring! (Ich red’ da lieber deutsch, aber jeder wie er/sie will).
Betrachten wir die Anforderungen:

  • beim Aufruf der Funktion getSession() wird eine Rückgabe vom Typ Boolean erwartet, dies sollten wir beachten.
  • Vereinfacht wird das refactoring (hier finde ich das passend) dadurch, dass es keine Übergabeparameter an die Funktion gibt.

So, die Hülle der Funktion steht, nun zum Inhalt.
Was macht die Funktion eigentlich?

  • Sie legt eine lokale Variable an, in der die aktuelle Zeit gespeichert wird.
  • Sie legt eine lokale Variable an, in der ein Wert aus der aktuellen Session gespeichert wird.
  • Wenn die aktuelle Zeit größer ist als der Wert der Session, gibt die Funktion false zurück.
  • Ist die aktuelle Zeit kleiner als der Wert der Session, dann gibt die Funktion true zurück.
  • Hier schon der erste Fehler: Was ist, wenn beide Werte gleich sind? Dieser Fall wird nicht berücksichtigt.

Nun das refactoring:

  1. Die Funktion getTimestamp() wird in der Funktion getSession() durch den Aufruf von time() ersetzt, somit fällt diese Funktion weg.
  2. Die Funktion getSession_Timeout() wird in der Funktion getSession() durch die Rückgabe von getSession_Timeout() ersetzt, somit fällt auch diese Funktion weg.
  3. Ziel der Funktion ist es, ein true oder false bei o.a. Bedingungen zurückzugeben.

Während unseres Refactorings verbessern wir auch gleich die Funktion der Funktion (*g*) und legen auch gleich fest, was passieren soll:

  • Ist die aktuelle Zeit größer wie der Wert in der Session, dann gibt die Funktion true zurück, sonst false.

Klingt einfach? Ist es auch.
Verkürzt man nun sukzessive die if…else Schleife und bedenkt, dass man die beiden lokalen Variablen nicht braucht und ersetzt diese mit ihren Entsprechungen (time() und Session-Wert),  so erhält man folgende Funktion:

function getSession()
{
  return (time() < $_SESSION[‚usrTimeout‘]);
}

Kurz, knackig und präzise. Diese Funktion erfüllt alle Anforderungen (und übererfüllt sogar diese, in dem es die alte Funktion um den Bereich “Zeit ist gleich Sessionwert” erweitert).

Für meine Behauptung oben auf die Frage “Wieviel Code davon ist nötig” würde ich nun sagen: q.e.d.

So, das war ein kleiner Ausflug in die Welt des Refactoring.
Ich hoffe, ihr habt alle was gelernt …