Uff. Ich hoffe, ich bekomme keine Bauchschmerzen beim Versuch, das zu verdauen 😀
Danke Dedlfix für die erste Feedback-Runde.
Das Singleton-Pattern hat hier einen besonderen Nachteil: Damit es greift, muss deine ChangeLogReader Klasse explizit Config::getData() aufrufen. Es hat also eine feste Verbindung zur Config-Klasse. Sowas versucht man, mit Dependency Injection zu vermeiden.
Grundsätzlich ist es ja immer so, dass dein PHP Hauptprogramm erstmal prozedural startet. D.h. um mit OOP loszulegen, musst Du erstmal irgendein Objekt erzeugen und darauf eine Methode aufrufen. Niemand verbietet Dir, das mehrfach zu tun, und den allgemeinen Setup kannst du, wie ich finde, gerne prozedural erledigen. Z.B. so:
$params = new UrlParams(); $config = new Config($_SERVER['DOCUMENT_ROOT'] . '\config\config.json');
$config->SelectLanguage($params->language); // siehe Unten
$logContainer = new ChangeLogContainer($config); // Injizierte Abhängigkeit! $logContainer->loadChangeLogs();
$app = new App($config, $logContainer); $app->ExecuteRequest($params);
Die Konstruktoren von Config, UrlParams und ChangeLog können gerne alles tun, was nötig ist, um Konfiguration, URL-Parameter und die Sammlung der Logfiles bereitzustellen. Da Du das Lesen der einzelnen ChangeLog Datei mit simplexml durchführst, hast Du eigentlich gar keinen Reader, die Hauptaufgabe der Klasse ist eher, einen Container für alle geladenen XML Dateien bereitzustellen. Da das Laden der ChangeLogs eher umfangreich ist, hatte ich so das Gefühl, als sollte man das nicht im Konstruktor machen. Ich bin mir aber nicht sicher. Prinzipiell könnte der Konstruktor es mit erledigen. Vielleicht haben die Kollegen dazu eine andere Meinung.
Achso - hast Du simplexml_load_file gesehen? Da kannst Du file_get_contents und simplexml_load_string zusammenfassen.
Bei UrlParms bin ich Dedlfix' Vorschlag gefolgt, die Methoden wegzulassen. Der Konstruktur kann die drei Parameter, die für deine Anwendung relevant sind, in Properties von UrlParms ablegen. Er sollte sie aber vorher noch inhaltlich validieren, also z.B. keine ungültige Sprache zulassen. Es gibt andere Anwendungen, wo die URL Parameter sehr variieren oder auch POST Requeste mit urlencoded Daten im Requestbody verarbeitet werden, da müsste man vermutlich ein anderes Interface zu den Requestparametern bauen. Für deinen Einsatzzweck dürfte diese Klasse hinreichend sein. Was PHP nicht hat, sind readonly-Properties; wenn du sicherstellen willst, dass die URL Parameter durch deinen Code unveränderlich sind, musst Du sie in get-Funktionen kapseln. Vertraust Du Dir? ;-)
Deiner Config-Klasse würde ich noch zwei Methoden verpassen: SelectLanguage($lang), die eine private Variable im Config-Objekt setzt, und eine Methode GetText($textCode) verpassen, die Dir zu einem TextCode den konfigurierten Text zur ausgewählten Sprache zurückgibt. Ob du die ausgewählte Sprache ständig in der URL mitschleppst, oder ob du sie in einem Cookie speicherst, kannst Du Dir noch überlegen.
Für den weiteren Ablauf deiner Anwendung frage ich mich jetzt, woher die ExecuteRequest-Methode Application-Klasse wissen soll, was zu tun ist. Beim Erstaufruf wird sie vermutlich eine Übersicht der geladenen ChangeLog-Dateien anzeigen und pro Datei einen "Anzeigen" Button bieten, der die Seite mit dem Hash dieser Application neu aufruft.
Dazu bräuchtest Du nun als nächstes eine Klasse LogIndexView - oder so. Alle Klassen, deren Job es ist, eine HTML Seite zu erzeugen, auf ein spezifisches Suffix wie 'View' enden lassen, kann der Übersicht dienen. Für ein vollständiges MVC Konzept müsstest Du nun noch einen LogIndexController bauen, der die eigentliche Arbeit macht.
Application::ExecuteRequest hat die Aufgabe des Routers. In deinem Fall:
if ( /* ist AppHash vorhanden */ )
{
/* Changelog zu diesem Hash anzeigen */
}
else
{
$controller = new LogIndexController($config);
$controller->Show(new LogIndexView());
}
Der Controller extrahiert aus den geladenen ChangeLogs die Daten, die zur Anzeige des Index nötig sind, und übergibt sie dann zur Darstellung an den View. Dazu überlegst Du Dir am besten ein einheitliches API für Views, so dass Views austauschbar werden. Natürlich müssen sich Controller und View über die Struktur der anzuzeigenden Daten einig sein - es macht keinen Sinn, einen LogIndexView an einen ShowChangeLogController zur Anzeige zu übergeben.
Ob Du dem Controller den View injizierst - wie hier - oder der Controller entscheiden muss, welcher View zu verwenden ist, das hängt vom Anwendungsfall ab. Manchmal kann der Router den View festlegen, nicht immer.
An dieser Stelle zeigt sich, dass dein eifriges Laden aller ChangeLogs unnötig sein kann - den Aufruf von "getChangeLogs", den ich oben noch im Init stehen habe, gehört vielmehr in die Show-Methode des LogIndexControllers. Ein ShowChangeLogController muss ja nur eine einzige ChangeLog-Datei laden. Wie Du dann aus dem Hash auf die Changelog-Datei kommst, ist eine andere Frage :-))
Hoffe, damit kommst Du erstmal weiter, ich feiere jetzt weiter Karneval…
Rolf