AND an Funktion übergeben

[Rolf B]

Hallo Meowsalot,

ich krieg die Krise wenn ich versuche, diesen Codeblob zu verstehen. Und ich habe in meinen 30 Jahren im Beruf schon einiges an Quälcode erlebt.

1. „Die Funktion hat mittlerweile fast 300 Zeilen Code.“

Die Funktion ist zu groß. Sie macht garantiert mehr als eine Sache. Zerlege sie in Funktionsblöcke. Ein paar Zeilen kannst Du auch eindampfen, wenn Du weiterliest...

2. Dynamisches Binden

Ich würde wetten, dass Du das Statement nach Bedarf zusammenhämmerst und dann genau einmal ausführst. Richtig?

Dann hast Du von einem Prepare überhaupt keinen Nutzen. Sind deine $values-Einträge eigentlich immer diese Hex-Strings und vertrauenswürdig (d.h. irgendwann mal durch eine Korrektheitsprüfung gelaufen)? Dann ist es doch das einfachste, sie mit

$empfaengerListe = "'".implode("','", $values)."'";

zu einem String zusammenzusetzen. Und an Stelle einer umständlich erzeugten Fragezeichenliste setzt Du einfach die $valueList in die Klammer vom INs SQL Statement. Schwups, ist die dynamische Bindung verschwunden.

Wenn die $value-Werte nicht vertrauenswürdig sind oder Hochkommas enthalten können, musst Du sie natürlich erstmal korrekt für den Kontextwechsel ins SQL escapen. Das weißt Du besser als wir.

Den prepare brauchst Du dann nur noch für $von und $an, mit zwei s-Parametern.

3. varvar Akrobatik

Hatte ich das nicht schonmal angesprochen? Diese Mühe ist nicht nötig. Man kann auch Referenzen auf Array-Elemente bilden und in einem Array abspeichern. Dazu ist es nur nötig, die Variable der foreach-Schleife als Referenz zu definieren und dann nochmals als Referenz ins $params Array zu legen. Aber wenn Du den Inhalt des IN nicht zu binden brauchst, ist diese Übung ohnehin nicht nötig.

foreach ( $values as &$v ) {    // Hier ein & vor $v
   $params[] = &$v;             // und hier auch noch mal
}

Alternativ so, ohne Zwischenreferenzen (aber mit Zählvariable). Ich laufe abwärts um nicht pro Durchlauf count($values) neu bilden zu müssen

for ($i = count($values)-1; $i >= 0; $i--) {
   $params[$i] =& $values[$i];
}

4. Die $select-Variable

Steht in $select eigentlich das gleiche wie zwischen UNION und WHERE? Ich würde es stark vermuten. In dem Fall kannst Du doch $select zweimal verwenden, oder?

5. Ist der UNION nötig?

Wenn man annimmt, dass Nr. 4 zutrifft, müsste man die beiden WHERE-Bedingungen auch zusammenlegen können, statt einen UNION zu bilden. Wenn nicht, ok, dann geht's nicht. Aber Du könntest dann Bausteine bilden. Die Abfragen auf status und bereichEmpfaenger sind in beiden Teilen gleich. Die kannst Du in eine Variable $filter legen und diese Variable in den SQL String einsetzen.

Mit UNION:

   $empfaengerListe = "'" . implode("','", $values) . "'";
   $statusListe = "'d9788f30bcf311ed98ef6bd5113784b2', '302fa36fca330e8faf9a5fe9f6ca5637'";
   $filter = "status NOT IN ($statusListe) AND bereichEmpfaenger IN ($empfaengerListe)";

   $sql = "$select
           WHERE $filter AND vertraulich = '0'
           UNION
           SELECT ... 
           FROM ... JOINJOINJOIN
           WHERE $filter AND vetraulich = '1' AND (apSender = ? OR apEmpfaenger = ?)
           ORDER BY erstellungsdatum DESC";

   $stmt = $mysqli->prepare($sql);

Falls $select und der zweite SELECT identisch sind, kannst Du den UNION zu einem OR machen:

   $empfaengerListe = "'" . implode("','", $values) . "'";
   $statusListe = "'d9788f30bcf311ed98ef6bd5113784b2', '302fa36fca330e8faf9a5fe9f6ca5637'";

   $sql = "$select
           WHERE status NOT IN ($statusListe)
           AND bereichEmpfaenger IN ($empfaengerListe)
           AND (   vertraulich = '0'
                OR vertraulich = '1' AND (apSender = ? OR apEmpfaenger = ?) )
           ORDER BY erstellungsdatum DESC";

   $stmt = $mysqli->prepare($sql);

Die Bausteine machen das ganze lesbarer und leichter debug-bar. Finde ich. Die Trennung in Konstruktion des SQL und Übergabe an Prepare hat noch den Vorteil, dass Du im Zweifelsfall mit var_dump das erzeugte SQL betrachten kannst.

Rolf