Kategorien
PHP Code Smells – Include
Heute wollen wir mal wieder eine neue kleine Kategorie starten. Es geht um Code Smells im PHP Umfeld. Code Smells bezeichnen dabei stellen im Code, an denen man ahnen kann, dass etwas nicht ganz so sauber gelöst ist. Die Stelle stinkt also. Der Begriff wurde von Kent Beck (oder eher von seiner Oma) geprägt und von Martin Fowler in seinem Buch über Refactoring wunderbar verwendet. Es lohnt sich übrigens das Buch zu besitzen, aber ich glaube das hab ich schon mal erwähnt.
Bestimmt kennt jeder von euch solche Muster, die er in seinem Code lieber nicht finden will. Heute will ich mal mit dem „Include Smell“ anfangen. Wenn ich ein größeres Projekt starte, dann will ich dies objektorientiert angehen. Alle Logik ist also in Klassen verpackt und keine „losen“ Aufrufe liegen irgendwo rum. Zumindest stelle ich mir das so vor. Wenn ich strikt mit Klassen arbeite, dann verwende ich include_once oder require_once, denn jede Klasse muss ich dem System gegenüber nur ein mal vorstellen und kann es dann nutzen. Ich brauche also auf keinen Fall die Möglichkeit eine Datei doppelt zu inkludieren.
Jedes vorkommen von include, das in der Business-Logik vorkommt, lässt also auf eine kaputte OOP schließen. Solche Stellen kann man also suchen und analysieren. Gerne kann man auch einen PHP_CodeSniffer Sniff verfassen, ob man aber so weit gehen muss, weiss ich nicht. Ich habe übrigens explizit diesen Smell auf die Busines Logik begrenzt, da man, glaube ich zumindest, im Templating System also der View auf solche includes nicht verzichten will. Wie seht ihr das? Ist jedes vorkommen eines includes, das nicht einfach durch ein include_once ersetzt werden kann ein schlechtes Zeichen und stinkt?
Wahrscheinlich werden diese Code Smell all denen liegen, die auch mit Metriken etwas anfangen können, weil beides in Richtung statische Code-Analyse zielt. Wer Metriken nicht aussagekräftig findet, der wird wahrscheinlich auch mit den Code Smells nicht so viel anfangen können. Auf jeden Fall werde ich in den nächsten Tagen noch was über mixed als Rückgabewert schreiben und auch ein wenig meine Aussage über Kommentare im Code von damals verfeinern. Ach ja die übermäßige Verwendung von Arrays wird auch noch ein Smell werden.
PS: Es kann übrigens sein, dass ich schon mal einen ähnlichen Artikel verfasst habe, aber ich wollte im Zuge der Code Smell Reihe noch einmal das ganze auffrischen und mir war da einfach nach.
Mist … doofe „Lesetipps“ naja ein paar neue Aspekte habe ich ja reinbekommen. Aber keine Sorge, ich fange jetzt nicht an meine Artikel alle neu zu schreiben.
Vielleicht wären noch Hinweise auf __autoload() und spl_autoload_register() angebracht: Damit werden include/require’s nur dann gemacht, wenn sie auch wirklich gebraucht werden.
Und ich muss nicht ewig include()’s tippen.
Damit lassen sich dann auch so schöne hierarchische Klassenstrukturen wie bei Kohana (http://v3.kohanaphp.com/guide/start.filesystem) und anderen Frameworks einfach und flexibel realisieren.
Du weißt aber schon, daß _once selbst ein Bad Smell ist oder?
@Christian: Das stimmt natürlich, falls man autoload nutzen kann, sollte man es auch nutzen. Ein lazy loading kann schon einiges an Performance rausholen. Weiß auch nicht, warum ich das vergessen habe. Danke.
@Sebastian: Was meinst du mit _once ist ein smell? Wolltest du auch auf die __autoload Methode hinaus?
TYPO: Wenn ich strikt mir Klassen arbeite
Vielleicht übersehe ich einen Aspekt, aber wenn ich ausschließlich mit Klassen arbeite, dann sollte ein Klassenloader auf jeden Fall implementiert werden. Gibt es einen guten Grund keinen klassloader zu schreiben ? (nehmen wir mal alte Projekte wo die Übersicht verloren gegangen ist heraus)
@Maren: Morderne Projekte sollte auch jeden Fall mit einem Classloader versehen werden. Da hast du auf jeden Fall recht. Ich habe aber schon öfters mit 3rd party Komponenten gearbeitet, die keine Struktur in ihren „Klassennamen“ hatten. autoload scheint also nicht zu 100% verbreitet zu sein.
Ich muss die Tage wohl einen autoload Artikel dazwischen schieben, um Missverständnisse auszuräumen.
Hmm, also ob ich das als smell bezeichnen mag? Es ist auf jedenfall ein Thema was man diskutieren sollte, aber include kann immer nut mittel zum Zweck sein. Was dann schlecht oder nicht schlecht ist lässt sich meiner Meinung nicht global sagen.
Include kann gut sein bei Template – Du schreibst es ja selber.
Include_Once kann Schrott sein (also nen Smell um im Wortlaut zu bleiben) wenn ich auf performance abzielen und das eh nur einmal aufgerufen wird.
SPL_Autoload ist da schon ein ziemlicher Königsweg gerade wenn ich protablen Code schreiben will der auch in fremden/neuen Umgebungen funktionieren darf.
Das mal nur so in den Topf geworfen. Man kann aus dem Thema sicherlich was machen.
Achso, PHP ist ne Scriptsprache. Das spielt bei includes sicherlich auch hier und da ne rolle. Ich kann ja auch in einen privaten Namensraum Includes einladen… . Da kommt ein Smell per Sniffer warscheinlich gar nicht mehr mit.
Irgendwie verstehe ich den heutigen Blog-Beitrag nicht. 🙁 Beispiel-Code wäre schon gewesen. Meinst du etwa anstatt Klassen voneinander erben zu lassen includieren Sie Teile von gleichen Klassenrümpfen? Wenn dem so wäre, ist das kein Bad-Smell sondern ein No-Go!
Mein klassischer Bad-Smell: Das Verwenden von Strg+C und Strg+V während des Programmierens.
@Nils
„Morderne Projekte sollte auch jeden Fall mit einem Classloader versehen werden“ + „keine Struktur in ihren “Klassennamen”“
…
Darüber könnte man sich vortrefflich streiten. Wissend, dass ich in der Minderheit bin, wenn ich diese neue „Autoload-Bewegung“ nicht gut finde, gibt es etliche Gründe dagegen, hier drei für mich essentielle (Anm.: ich bin DDD-ler, dort ist das Model der Code und umgekehrt und vor allem ist alles Genannte und Gesprochene auch im Code wiederzufinden. Ändere ich ein Konzept beim Namen, zieht das Model damit gleich):
– wer „intention revealing“ Klassennamen benutzt, ignoriert es völlig. Denn eine Klasse namens Zend_Controller_Action_Helper_AutoComplete_Abstract ist inakzeptabel.
– Eine Klasse mit obigem Namen müsste „umziehen“, sobald sich ihr Name oder ihre Funktion ändert. Wer nämlich mit logischen Patterns arbeitet, benennt ein Klasse nicht strikterweise „…Entity“ oder „…Repository“. Ganz im Gegenteil, dann hat man schon verloren, denn entscheidend ist eben nicht, wie eine Klasse technisch arbeitet oder welches Pattern ich benutze, sondern was sie macht und warum.
– Namespaces, zumindest so wie sie PHP implementiert, können (und sollten) Abhilfe schaffen. Allerdings vergewaltigen hier auch einige derzeit populäre Projekte dies wiederum bis zur Unkenntlichkeit.
Ich verwende in der zentralen Konfigurationsdatei immer die __autoload Methode, damit hat sich das Problem automatisch erledigt.
Hi,
da gibt es schon lange eine Frage die mich quält.. 😉
wird denn die performance sinken, wenn ich viele include_once aus verschiedenen Klassen auf die gleiche Datei verwende?
Sowas sollte doch PHP dann schon merken, oder?
Und wäre es besser alle include_once in den headder jeder Klasse zu schreiben (wie z.B. bei Java) oder nur in jeder Funktion wenn ich sie eben brauche?
Oder lade ich einfach mal alle Klassen die ich brauche auf einmal und kann diese dann benutzen wo ich will.?
@Andreas: generell ist include_once ein Performance Killer. Jede Form von includes oder require kostet einen Haufen Leistung, insofern lohnt es sich generell nur das zu inkludieren, was an wirklich braucht.
Das magische Stichwort hierbei ist ein Autoloader.
Um deine Frage zu beantworten: wenn du eine Datei aus mehreren „Pfaden“ per include_once inkludierst, ist das egal, genau dafür ist ja das include_once, anstelle vom include. Allerdings wird das Überprüfen, ob die Datei schon geladen wurde auch etwas Leistung kosten, sicher aber nich soviel, als wenn die Datei nochmal vom Dateisystem gelesen werden müsste.
Wobei mich dabei noch eine andere Frage quält: wofür benötigst du includes IN deinen Klassen?
ich habe mir eine klassenloader geschrieben, der ein array mit allen schon mal geladenen klassen enthält. Allerdings muss die Klasse ja auch abgearbeitet werden. Da war eben die Frage ob ein include_once besser/schneller als ein „ClassLoader:load(‚file.php‘);“ ist.
Zu deiner Frage:
normalerweise include ich alle Dateien die ich in einer Klasse brauche ganz oben. Manchmal habe ich auch eine Klasse, die nur in einer einzigen funktion eine andere Klasse braucht. z.B. eine Umrechnung o.ä.
Nach meinem standard vorgehen, lade ich die Util-Klasse immer.
Auch wenn meine benutzte klasse die funktion gar nicht ausführen würde.
jetzt probiere ich mal, ob ich hier auch Code beschreiben kann… zur darstellung:
include_once (util.php);
class foo{
function bar1($string){
echo 2
}
function bar2($int){
echo Util:intToString(2);
};
}
so wird die util classe immer geladen.
Wenn ich das include in die function bar2 setzte,
muss ich die datei nie laden, wenn ich nicht die funktion bar2 benutze.
ich hoffe, das war einigermassen verständlich.