createFieldMap() – die God Function

Wenn eine Funktion zu viel weiß, zu viel tut und zu viel bedeutet

Im Artikel über die Helper-Funktionen haben wir common_helper.php als Ganzes betrachtet – 130+ globale Funktionen, über 11.000 Aufrufe im Code, ein Relikt aus der prozeduralen Ära. In einem anderen Artikel haben wir LimeMailer als GodClass analysiert: eine Klasse die zu viel weiß, zu viel tut, und deshalb praktisch nicht testbar ist.

Heute schauen wir auf etwas das noch subtiler ist: eine GodFunction.

Keine Klasse. Eine einzelne Funktion. Mit über 600 Zeilen Code.


Der erste Blick

function createFieldMap(
    $survey,
    $style = 'short',
    $force_refresh = false,
    $questionid = false,
    $sLanguage = '',
    &$aDuplicateQIDs = array()
)

Sechs Parameter. Einer davon als Referenz übergeben – &$aDuplicateQIDs. Das ist bereits ein Warnsignal: die Funktion schreibt nicht nur in einen Rückgabewert, sondern auch in einen Parameter des Aufrufers.

Der Name klingt harmlos: erstelle eine Feldkarte. Eine Übersicht aller Felder einer Umfrage. Was kann daran schon kompliziert sein?


Was die Funktion wirklich tut

Fangen wir mit dem an, was direkt sichtbar ist.

Session-Caching:

if (isset(Yii::app()->session['fieldmap-' . $surveyid . $sLanguage]) 
    && !$force_refresh 
    && $questionid === false) {
    return Yii::app()->session['fieldmap-' . $surveyid . $sLanguage];
}

Die Funktion cached ihr Ergebnis selbst – in der Session. Sie entscheidet also nicht nur was sie zurückgibt, sondern auch wann sie es neu berechnet und wo sie es zwischenspeichert.

Zwei Datenbankabfragen – Raw SQL:

$defaultsQuery = "SELECT a.qid, a.sqid, a.scale_id, a.specialtype, al10.defaultvalue"
    . " FROM {{defaultvalues}} as a "
    . " JOIN {{defaultvalue_l10ns}} as al10 ON a.dvid = al10.dvid "
    . " AND b.same_default=0"
    . " AND b.sid = " . $surveyid;

Nicht ActiveRecord. Raw SQL, direkt in der Funktion. Und nicht eine Abfrage – zwei. Einmal für sprachspezifische Standardwerte, einmal für Standardwerte der Basissprache. Mit eigener Merge-Logik dazwischen.

Dann die Hauptabfrage – ein JOIN über vier Tabellen:

$aquery = "SELECT g.*, q.*, gls.*, qls.*"
    . " FROM $quotedGroups g"
    . ' JOIN {{questions}} q on q.gid=g.gid '
    . ' JOIN {{group_l10ns}} gls on gls.gid=g.gid '
    . ' JOIN {{question_l10ns}} qls on qls.qid=q.qid '
    . " WHERE qls.language='{$sLanguage}' and gls.language='{$sLanguage}'...";

Wer genau hinschaut bemerkt etwas: die Tabelle {{questions}} wird mit dem Alias q definiert. Aber in der optionalen Bedingung für den Question Preview wird questions.qid referenziert – nicht q.qid:

if ($questionid !== false) {
    $aquery .= " and questions.qid={$questionid} "; // <-- Bug: muss q.qid sein
}

Das ist ein echter Bug. Reproduzierbar mit dem generierten SQL in phpMyAdmin:

SELECT g.*, q.*, gls.*, qls.*
FROM lime_groups g
JOIN lime_questions q ON q.gid = g.gid
JOIN lime_group_l10ns gls ON gls.gid = g.gid
JOIN lime_question_l10ns qls ON qls.qid = q.qid
WHERE qls.language = 'de'
AND g.sid = 123456
AND q.parent_qid = 0
AND questions.qid = 42

MySQL antwortet unmissverständlich:

#1054 - Unbekanntes Tabellenfeld 'questions.qid' in where clause

Gemeldet: Bug #20464 im LimeSurvey Bugtracker.

Der Fix wäre eine einzige Zeile:

// Buggy:
$aquery .= " and questions.qid={$questionid} ";

// Korrekt:
$aquery .= " and q.qid={$questionid} ";

Warum ist dieser Bug nie aufgefallen? Weil $questionid !== false ausschließlich beim Question Preview gesetzt wird – einem Sonderfall den die meisten Entwickler selten nutzen. Die Hauptpfade – Survey-Aktivierung, Frontend-Rendering, Export – laufen alle mit $questionid = false und treffen diese Zeile nie.

Das ist ein klassisches Muster in GodFunctions: die Hauptpfade funktionieren zuverlässig. Die Sonderfälle verrotten still – ohne Test, ohne Alarm, ohne das berühmte Zittern.

Danach: die Fragetyp-Behandlung. Für jeden Fragetyp andere Logik:

Und am Ende: Randomisierung:

if (isset(Yii::app()->session['survey_' . $surveyid]['fieldmap-' . $surveyid . '-randMaster'])) {
    $masterFieldmap = Yii::app()->session['survey_' . $surveyid]['fieldmap-' . $surveyid . '-randMaster'];
    // ... merge logic für randomisierte Feldreihenfolge
}

Die Funktion weiß auch über Randomisierungsgruppen Bescheid.


Der $style-Parameter – eine versteckte Spaltung

Das subtilste Problem ist $style. Er kann 'short' oder 'full' sein – und diese Unterscheidung durchzieht die gesamte Funktion wie ein roter Faden:

if ($style == "full") {
    $fieldmap[$fieldname]['title'] = $arow['title'];
    $fieldmap[$fieldname]['question'] = $arow['question'];
    $fieldmap[$fieldname]['group_name'] = $arow['group_name'];
    $fieldmap[$fieldname]['mandatory'] = $arow['mandatory'];
    $fieldmap[$fieldname]['encrypted'] = $arow['encrypted'];
    $fieldmap[$fieldname]['hasconditions'] = $conditions;
    $fieldmap[$fieldname]['usedinconditions'] = $usedinconditions;
    $fieldmap[$fieldname]['questionSeq'] = $questionSeq;
    $fieldmap[$fieldname]['groupSeq'] = $groupSeq;
}

Dieses Muster wiederholt sich über ein Dutzend Mal in der Funktion. Jedes Mal dasselbe: im full-Modus werden zusätzliche Metadaten befüllt, im short-Modus nicht.

Das ist keine Optimierung. Das ist eine versteckte Aufteilung in zwei Funktionen die nie gemacht wurde. createFieldMapShort() und createFieldMapFull() existieren de facto – aber als ein einziger Monolith verpackt.


Der Kommentar der alles sagt

Mitten in der Funktion, bei der Behandlung von Conditions:

// Condition indicators are obsolete with EM. However, they are so tightly 
// coupled into LS code that easier to just set values to 'N' for now 
// and refactor later.
$conditions = 'N';
$usedinconditions = 'N';

"Refactor later."

git blame liefert die Antwort auf die Frage wie lange dieser Kommentar schon dort steht: 13 Jahre. Seit 2012. Er hat PHP 5.x, PHP 7.x und PHP 8.x überlebt. Er hat die Einführung von Bootstrap, das Plugin-System und unzählige Feature-Iterationen überlebt. Er ist kein Versprechen mehr. Er ist ein Fossil.

Und er zeigt das eigentliche Problem einer GodFunction: sie ist so tief ins System eingewachsen, dass selbst bekannte Verbesserungen nicht umgesetzt werden. Nicht weil niemand es will. Sondern weil die Kopplung jeden Eingriff zum Risiko macht.


Wo createFieldMap() überall aufgerufen wird

Das ist vielleicht das Beunruhigendste. Die Funktion ist nicht irgendwo – sie ist überall:

Das bedeutet: wer createFieldMap() anfasst, fasst gleichzeitig Survey-Aktivierung, Frontend-Rendering, Expression Manager, Export und Statistik an. Das ist Cognitive Load in seiner reinsten Form.


Was eine GodFunction von einer großen Funktion unterscheidet

Nicht jede lange Funktion ist eine GodFunction. Der Unterschied liegt nicht in den Zeilen – er liegt in den Verantwortlichkeiten.

createFieldMap() ist verantwortlich für:

  1. Caching – Session-Management, Cache-Invalidierung
  2. Datenbankzugriffe – drei Abfragen, eigene SQL-Strings
  3. Sprachlogik – Fallback auf Basissprache, Übersetzungsmerge
  4. Fragetyp-Behandlung – acht verschiedene Verzweigungen
  5. Feldname-Konstruktion – das SIDxGIDxQID-Schema
  6. Defaultwert-Logik – sprachspezifisch und basissprachlich
  7. Randomisierung – Merge mit randomisierter Reihenfolge
  8. Duplikat-Erkennung – via Referenzparameter

Das sind acht Verantwortlichkeiten. Das Single Responsibility Principle – ein Grundsatz aus SOLID – besagt: eine Funktion oder Klasse soll genau einen Grund haben, sich zu ändern.

createFieldMap() hat acht.


Screaming Architecture – oder ihr Fehlen

Robert C. Martin beschreibt Screaming Architecture als das Ideal: eine Codebasis soll schreien was sie tut. Wer in die Verzeichnisstruktur schaut, soll sofort verstehen worum es geht.

createFieldMap() schreit nichts. Sie murmelt.

Der Name verspricht eine einfache Abbildung – eine Karte der Felder. Was sie liefert, ist ein komplexes Array das je nach $style-Parameter fundamental anders aussieht, aus Session-Daten oder Datenbankabfragen befüllt wird, Randomisierung berücksichtigt, Duplicates meldet und Sprachfallbacks verwaltet.


Der Weg heraus: ein eigenes Composer Package

Die Domänenlogik von createFieldMap() ist wertvoll – sie ist nur falsch verpackt. Im Kern ist die Funktion ein Survey Field Schema Builder: sie nimmt eine Umfrage-Struktur und erzeugt daraus eine normalisierte Feldbeschreibung. Das ist eine abgeschlossene, testbare Aufgabe.

Die konsequente Lösung wäre ein eigenes Composer Package:

limesurvey/survey-fieldmap

Mit einer sauberen, ausdrucksstarken API:

$fieldmap = FieldMapBuilder::create($survey)
    ->withLanguage('de')
    ->withStyle(FieldMapStyle::FULL)
    ->withCache($sessionCache)
    ->build();

Was das Package kapseln würde:

Was es nicht mehr enthalten würde:

Der eigentliche Gewinn wäre nicht mal die Kapselung – sondern die Testbarkeit. Und nicht nur irgendeine Testbarkeit: ein isoliertes Package mit rein funktionaler Logik – Survey rein, Feldkarte raus – ist ein idealer Kandidat für 100% PHPUnit-Coverage. Keine Session-Magie, kein Yii::app(), keine globalen Abhängigkeiten die gemockt werden müssten.

Die vollständige QA-Kette wäre realistisch erreichbar:

Das Package würde damit denselben Qualitätsstandard erreichen wie Yii 3 selbst – das Framework das LimeSurvey nie adoptiert hat. Ein kleines, sauberes Stück modernes PHP, mitten in einer Legacy-Codebasis.

Das ist kein Traum – das ist das was Yii 3 und moderne PHP-Architektur als Standard setzen. Und es ist der Weg den Paul M. Jones in Modernizing Legacy Applications in PHP beschreibt: nicht alles auf einmal, sondern eine Verantwortlichkeit nach der anderen aus dem Monolithen herauslösen – so lange, bis der Monolith aus lauter sauberen Paketen besteht.


Was die LimeSurvey-Entwickler selbst sagen

Das Bemerkenswerteste ist vielleicht das: das Problem ist bekannt. Nicht nur von außen beobachtet – sondern intern dokumentiert.

Im LimeSurvey Code Quality Guide, der internen Entwicklerdokumentation, steht zu createFieldMap() wörtlich:

"Very many boolean arguments. Should probably be a class with methods like $fieldmapCreator->setForceRefresh(true); instead."

Und beim Aufruf im Code:

"$fieldMap = createFieldMap($survey, 'short', false, false, $language); // Same, make a DTO"

DTO – Data Transfer Object. Das ist die richtige Antwort. Ein typisiertes Objekt statt eines magischen Arrays das je nach $style-Parameter unterschiedliche Keys hat.

Die Diagnose ist also gestellt. Seit Jahren. Im eigenen Haus.

Das macht createFieldMap() zu einem besonders lehrreichen Beispiel: nicht weil niemand das Problem sieht, sondern weil das Sehen allein nicht reicht.


Warum sie nicht einfach refactored werden kann

Die ehrliche Antwort: weil sie zu viele Aufrufer hat, die zu viel über ihre Interna wissen.

Aufrufer gehen davon aus dass $style = 'full' bestimmte Keys liefert. Andere bauen auf $style = 'short' und den Session-Cache. Wieder andere übergeben $questionid für Previews. Der Expression Manager post-prozessiert das Ergebnis mit Annahmen über die Array-Struktur.

Ein Refactoring von createFieldMap() ist kein lokaler Eingriff. Es ist eine koordinierte Migration durch den gesamten Anwendungskern.

Das ist der Preis einer GodFunction. Nicht die Komplexität beim Schreiben – sondern die Kopplung die sie hinterlässt.


Fazit

createFieldMap() ist das Herzstück der LimeSurvey-Datenverarbeitung. Ohne sie läuft keine Umfrage, kein Export, keine Statistik. Sie ist seit über zehn Jahren im Einsatz, durch unzählige PHP-Versionen, Framework-Upgrades und Feature-Iterationen hindurch.

Das ist ihre Stärke. Und ihr Problem.

Eine Funktion die alles kann, kann nicht ersetzt werden ohne alles zu kennen. Sie ist nicht testbar in Isolation. Sie ist nicht verstehbar ohne Kontext. Und sie wächst weiter – weil jede neue Anforderung die irgendwie mit Feldern zu tun hat, am Ende wieder hier landet.

Der Kommentar "refactor later" aus dem Jahr 2012 ist nicht zynisch gemeint. Er ist ehrlich. Manchmal ist later die einzig realistische Antwort.

Aber later braucht einen Plan. Und der Plan beginnt damit, zu verstehen was man vor sich hat.

Das ist createFieldMap().