Im zweiten Teil dieser Geschichte ging es um eine unbequeme Erkenntnis: Zwei zehn Jahre lang getrennt gewachsene Statistics-Implementierungen lassen sich nicht einfach wieder zusammenführen. Ein Diff zeigt, dass sich Admin und Public Statistics unterscheiden – er sagt nicht, ob dieser Unterschied fachlich notwendig ist oder nur historische Drift.
Also erst verstehen, dann verändern. Characterization Tests, um vorhandenes Verhalten zu dokumentieren. Die versteckte $_POST-Abhängigkeit sichtbar machen, als ersten kleinen Schritt zu einer Trust Boundary.
Das führt zu einer neuen Frage, mit der Teil 2 endete: Wir haben verstanden, was gemeinsam sein sollte. Aber wie verhindert man, dass ein neuer gemeinsamer Kern mit der Zeit wieder von Yii, HTTP, Session, SQL und Rendering durchwachsen wird – also genau dasselbe Schicksal erleidet wie die beiden Helper, die wir eigentlich ablösen wollen?
Auch hier gilt derselbe Vorbehalt wie in Teil 2: Was folgt, ist mein Vorschlag, keine dokumentierte Faktenlage.
Eine Grenze, die man dokumentieren kann – oder eine, die man erzwingt
Die naheliegende Antwort wäre, einen neuen Ordner anzulegen, etwa application/services/statistics/, und dort die gemeinsamen Klassen zu sammeln. Das wäre bereits eine Verbesserung.
Aber bei einer gewachsenen Anwendung neigen Grenzen, die nur in Köpfen und Architekturdiagrammen existieren, dazu, zu erodieren. Heute entsteht ein sauberer StatisticsCalculator. Morgen braucht jemand unter Zeitdruck schnell den aktuellen Survey und greift auf Yii::app()->getController()->getSurvey() zu. Übermorgen wird eine Session gebraucht, dann ein ActiveRecord, dann ein View. Ein paar Jahre später existiert ein neuer Statistics-Helper, diesmal nur mit modernerem Namen, aber denselben Abhängigkeiten wie die beiden alten.
Genau das ist ja der Ausgangspunkt dieser ganzen Geschichte: 2016 wollte auch niemand zwei Statistics-Implementierungen. Es entstanden trotzdem zwei.
Wenn mir die Grenze wichtig ist, würde ich sie deshalb nicht nur dokumentieren, sondern technisch erzwingen. Konkret würde ich einen gemeinsamen Statistics-Core als internes Composer-Package innerhalb des LimeSurvey-Repositories anlegen – nicht als eigenständiges Open-Source-Projekt, nicht auf Packagist, ohne eigenen Release-Zyklus. Das Package wäre zunächst kein Produkt. Es wäre eine Architekturgrenze.
LimeSurvey Application
│
├── Admin Statistics
│
├── Public Statistics
│
├── RemoteControl
│
└───────────────┐
│
▼
statistics-core
│
├── Filter
├── Validation
├── Calculations
└── Result Models
Diese Grenze bekäme klare Regeln. Der Core dürfte PHP, Value Objects, DTOs, Filter, statistische Berechnungen und Result Models kennen. Er dürfte nicht kennen: Yii, $_POST, Session, Controller, ActiveRecord, HTML, Views, PDF, Excel.
Der Unterschied zur reinen Ordnerstruktur ist, dass diese Grenze nicht mehr nur von Disziplin und Dokumentation abhängen würde. Wenn der Core Yii nicht als Abhängigkeit deklariert und seine Tests und statische Analyse isoliert gegen die Package-Abhängigkeiten laufen, wird ein versehentlicher Zugriff auf Yii::app() unmittelbar sichtbar – durch Tooling überprüfbar, nicht nur durch Review-Aufmerksamkeit.
Eine Qualitätsinsel, kein Großprojekt
Und hier passiert für mich das eigentlich Interessante: LimeSurvey komplett auf moderne Typisierung, maximale statische Analyse und umfassende Tests umzustellen, wäre ein eigenes, sehr großes Projekt. Das muss aber nicht die Voraussetzung sein.
Für einen neuen, klar abgegrenzten Core könnten andere Regeln gelten als für seine Umgebung: declare(strict_types=1), PHPUnit, PHPStan auf einem möglichst strengen Level, Coverage-Messung, explizite Typen statt beliebiger Arrays, wo ein Value Object die fachliche Bedeutung ausdrücken kann. Aus
calculate($fieldmap, $responses, $filter);
könnte so etwas werden wie
calculate(
SurveyDefinition $survey,
ResponseSet $responses,
StatisticsFilterSet $filters
): StatisticsResult;
Nicht weil moderne PHP-Syntax schöner aussieht, sondern weil an dieser Grenze Unklarheit teuer ist – genau das, was PR #5225 aus Teil 1 gezeigt hat. Die Legacy-Anwendung dürfte weiterhin Arrays, ActiveRecords und dynamische Strukturen besitzen. Ein Adapter würde diese Welt in die strengere Welt des Cores übersetzen. Man müsste nicht erst den gesamten Ozean trockenlegen, um eine einzelne Insel darauf zu bauen.
Coverage wäre dabei für mich kein Pokal, den man vorzeigt. Interessanter ist die Frage, welcher Code sich nicht unbemerkt verändern darf. Bei diesem Statistics-Core wären das vor allem Filterinterpretation, Feldvalidierung und die statistischen Berechnungen selbst – Dinge, die sich deterministisch testen lassen und deren Grenzfälle sich explizit dokumentieren lassen. Ein Adapter, der ein altes LimeSurvey-Array in ein DTO übersetzt, braucht vermutlich eine andere Teststrategie. Coverage wäre dann keine Zielzahl, sondern eine Karte, die zeigt, welches Verhalten bereits abgesichert ist und welches nicht.
Berechnen ist nicht darstellen
Ein ähnliches Problem liegt tiefer in den Statistics-Helpern. Methoden wie generate_statistics() sind historisch gewachsen und erledigen mehrere Dinge gleichzeitig: Daten abfragen, statistisch auswerten, formatieren, für unterschiedliche Ausgaben vorbereiten – HTML, PDF, Excel, Charts.
Das erklärt übrigens auch, warum Admin Statistics und Public Statistics legitimerweise unterschiedlich geworden sind: Sie dürfen unterschiedlich darstellen. Die eigentliche Frage ist nur, warum sie deshalb auch dieselben statistischen Berechnungen separat besitzen müssten.
Eine sauberere Trennung würde die Berechnung von der Darstellung lösen:
Response Data
│
▼
Statistics Core
│
▼
QuestionStatistics
│
┌──┼──────┬──────┐
▼ ▼ ▼ ▼
HTML PDF XLS Chart
Der Core würde ein Ergebnis liefern, das fachlich beschreibt, was berechnet wurde – Antworten mit Label, Anzahl und Prozentanteil, Gesamtsumme, Mittelwert, Standardabweichung, Quartile. Ob der Core dafür vollständige Response-Daten erhält, über ein Repository arbeitet oder vorberechnete Aggregationen verarbeitet, wäre eine eigene Designentscheidung. Entscheidend ist zunächst die Richtung der Abhängigkeiten, nicht die konkrete Schnittstelle. Ob daraus anschließend eine Admin-Tabelle, eine öffentliche Statistikseite, ein Excel-Sheet oder ein PDF wird, wäre ebenfalls nicht mehr seine Verantwortung. Die Unterschiede zwischen Admin und Public würden damit nicht verschwinden. Sie würden nur dort landen, wo sie hingehören.
Wer heute schon auf generate_statistics() zugreift
Eine solche Grenze lässt sich nicht in einem Zug einführen, ohne bestehende Nutzer zu gefährden – und Admin Statistics hat mehr Nutzer, als man auf den ersten Blick vermuten würde.
Ein Blick in remotecontrol_handle.php zeigt: Die RemoteControl-API instanziiert in ihrer export_statistics()-Methode direkt den Admin-Helper und ruft dessen generate_statistics() in drei Varianten auf – für PDF-, XLS- und HTML-Export.
$helper = new statistics_helper();
switch ($docType) {
case 'pdf':
$sTempFile = $helper->generate_statistics($iSurveyID, $aSummary, $aSummary, $graph, $docType, 'F', $sLanguage);
// ...
case 'xls':
$sTempFile = $helper->generate_statistics($iSurveyID, $aSummary, $aSummary, '0', $docType, 'F', $sLanguage);
// ...
case 'html':
$sResult = $helper->generate_statistics($iSurveyID, $aSummary, $aSummary, '0', $docType, 'DD', $sLanguage);
}
Das bedeutet: Ein Refactoring von generate_statistics() betrifft nicht nur das Admin-UI. Auch die RemoteControl-Implementierung hängt intern an diesem Einstiegspunkt. Seine Signatur ist damit zwar kein externer API-Vertrag, aber eine bestehende interne Integrationsgrenze, die bei einem Umbau berücksichtigt werden muss. Eine Methode wie diese eignet sich deshalb eher als vorübergehende Fassade denn als erstes Abrissobjekt.
Schrittweise statt auf einmal
Man könnte einen Branch eröffnen, statistics-core anlegen, beide Helper vollständig auseinandernehmen, sämtliche Queries umstellen, DTOs einführen, Rendering trennen – und nach einigen Wochen einen Pull Request mit mehreren tausend geänderten Zeilen präsentieren. Technisch vielleicht beeindruckend, praktisch kaum reviewbar, und für ein Subsystem mit zehn Jahren angesammelter Sonderfälle, das zudem über RemoteControl von außen genutzt wird, riskant.
Ich würde hier auf ein Strangler-Refactoring setzen, benannt nach dem Strangler-Fig-Muster: Eine neue Struktur wächst schrittweise um die alte herum und übernimmt nach und nach deren Verantwortlichkeiten, während die alte weiterläuft.
Heute:
statistics_helper
████████████████████
userstatistics_helper
██████████████
Nach ersten Extraktionen:
statistics_helper
██████████████
│
└────► statistics-core
userstatistics_helper
██████████
│
└────► statistics-core
Später:
Admin Adapter ───────┐
▼
statistics-core
▲
Public Adapter ──────┘
Die bestehenden Einstiegspunkte – auch generate_statistics() – könnten dabei zunächst erhalten bleiben. Von außen, auch für RemoteControl, würde sich vorerst nichts ändern:
$helper->generate_statistics(...);
Intern würde Verantwortung Schritt für Schritt in den neuen Core wandern. So würde aus einem Refactoring keine gleichzeitige Migration aller Aufrufer des Subsystems auf einmal.
Ein möglicher Fahrplan
Daraus ergibt sich für mich eine Reihenfolge – nicht als detaillierter Projektplan, sondern als Grundsatz:
1. Verhalten charakterisieren
↓
2. Unterschiede klassifizieren
↓
3. versteckte Dependencies sichtbar machen
↓
4. Trust Boundaries herstellen
↓
5. gemeinsamen fachlichen Kern bestimmen
↓
6. harte Package-Grenze schaffen
↓
7. gemeinsamen Code schrittweise extrahieren
↓
8. Legacy-Helper delegieren lassen
↓
9. Rendering und Adapter zurücklassen
↓
10. Legacy entfernen, wenn es nichts mehr zu tun hat
Die Reihenfolge ist mir dabei wichtiger als jeder einzelne Schritt. Das Package kommt nicht zuerst. Die Zielarchitektur kommt nicht zuerst. Nicht einmal die Code-Deduplizierung kommt zuerst. Zuerst kommt Verständnis – der ganze Gegenstand von Teil 2.
Was ich dabei ausdrücklich nicht tun würde
Vielleicht ist die Negativliste genauso wichtig wie der Fahrplan selbst.
Ich würde keine AbstractStatisticsHelper mit mehreren tausend gemeinsamen Zeilen bauen. Ich würde nicht automatisch die neuere Implementierung zur richtigen erklären. Ich würde keine Unterschiede vereinheitlichen, deren Bedeutung ich nicht verstanden habe. Ich würde keine Query-Logik ohne Characterization Tests verändern. Ich würde nicht versuchen, nebenbei ganz LimeSurvey zu modernisieren. Und ich würde aus statistics-core zunächst kein eigenständiges Produkt mit Packagist-Release, SemVer-Vertrag und eigenem Repository machen.
Die Package-Grenze soll dem Code dienen. Nicht umgekehrt.
Wann wäre das Refactoring fertig?
Wenn beide Helper verschwunden sind? Wenn kein duplizierter Code mehr existiert? Wenn PHPStan keine Findings mehr meldet? Wenn die Coverage 100 Prozent beträgt?
Nichts davon wäre für mich das entscheidende Kriterium. Erinnern wir uns daran, warum wir überhaupt hier gelandet sind: PR #5225 musste eine Erkenntnis über sichere Response-Spalten zweimal umsetzen. Das eigentliche Ziel wäre für mich deshalb erreicht, wenn eine zukünftige Erkenntnis dieser Art nur noch eine fachlich zuständige Stelle besitzt. Wenn LimeSurvey morgen feststellt, dass ein Statistics-Filter ein bestimmtes Response-Feld unter einer bestimmten Bedingung nicht verwenden darf, dann sollte es genau einen Ort geben, an dem diese Regel implementiert und getestet wird – den Admin Statistics, Public Statistics und RemoteControl gemeinsam verwenden. Nicht weil alle drei dasselbe sind, sondern weil diese eine Regel nur eine Bedeutung hat.
Das ist der Unterschied zwischen der Beseitigung von dupliziertem Code und der Beseitigung von dupliziertem Wissen.
Und square()?
Natürlich könnte man ganz am Anfang einfach die beiden square()-Funktionen zusammenführen. Der Diff wäre klein, das Risiko minimal, die Codebasis hätte danach ein paar Zeilen weniger, und es gäbe nur noch eine Funktion, die zuverlässig Zahlen quadriert.
Damit wäre aber noch nichts von dem gelöst, was square() überhaupt erst sichtbar gemacht hat. Vielleicht verschwindet sie irgendwann im Lauf eines solchen Refactorings. Vielleicht wird sie durch eine gewöhnliche Berechnung ersetzt. Vielleicht stellt sich heraus, dass sie längst niemand mehr braucht. Das spielt eigentlich keine Rolle.
square() war nie das Problem. Sie war nur das Fossil. Das eigentliche Problem war das Wissen, das vor zehn Jahren gemeinsam mit ihr kopiert wurde – und genau dort müsste ein Refactoring ansetzen.