Problemfall $dataset->getXyz()

[christophboecker]

Die Modelclasses haben für alle wichtigen Datenfelder eine Getter bzw. Setter. Beispiel:

class Author extends rex_yform_manager_dataset
{
    public function getName(): ?string
    {
        return $this->getValue('name');
    }

Die Abfolge

$autor = Author::create();
$name = $autor->getName();

liefert neben null als Rückgabewert auch die Meldung Warning: Undefined array key "name" in redaxo/src/addons/yform/plugins/manager/lib/yform/manager/dataset.php]) on line 290.

Zur Absicherung müsste erst einmal geprüft werden, ob ein Wert vorliegt ($autor->hasValue('name');). (Ob vom richtigen Typ sei mal dahingestellt; denn mit setValue kann man reinwuppen was man will. Das Fass möchte ich in diesem Issue nicht aufmachen.)

Mal angenommen, dass in $data['name'] ein nicht nach String umwandelbarer Inhalt steht, würde ich doch sehr stark annehmen, dass das nur über einen Entwicklerfehler passiert ist. In dem Fall würde ich mich auf den Standpunktstellen, dass ein Whoops angebracht ist, und folglich den Fehler nicht abfangen. Wenn er doch abgefangen werden sollte, müsste wohl ein Try-Catch her.

Foolproof sähe der Code eher so aus:

class Author extends rex_yform_manager_dataset
{
    public function getName(): ?string
    {
        if( self::hasValue('name') {
            try {
                return $this->getValue('name');
            } catch (Throwable $th) {
                // void; fängt nicht in string umwandelbare Feldinhalte ab
            }
        }
        return null;  // oder return '';
    }

Nun kann man noch drüber diskutieren, ob null als Rückgabe wirklich notwendig wäre oder statt dessen '' genommen wird. Denn dann müsste im Anwendungscode nur "leerer Name" und "Name" unterschieden werden und nicht zusätzlich auf "kein Name". Beide Varianten haben ihre Berechtigung.

Bevor ich hier x Methoden anfasse, sollte es ertmal eine Grundsatzentscheidung geben, ob das überhaupt und wenn ja wie angegangen werden sollte.

[alxndr-w]

Ist das nicht ein Fehler von YOrm und dessen getValue()-Methode?

[christophboecker]

Hm, nicht wirklich. Die Methode ist halt sehr unspezifisch. Und die Situation nach ::create ist halt schon immer so gewesen, dass der interne Speicher leer ist. Hier im Addon kommen ja auch nach einem ::create die nötigen setValue bzw. setXyz, so dass es wieder passt.

Ich denke in der Tat eher daran, dass man die eigenen Methoden recht simpel hält und das TryCatch-Gedöns weglässt - Developer Experience lässt grüßen. Die eigenen getXyz-Methoden könnte man mit ´nem Default-Wert aufpeppen.

class Author extends rex_yform_manager_dataset
{

    protected function getValueWithFallback(string $name, string|int|float|null $default = null): string|int|float|null
    {
        if( $this->hasValue('name') {
                return $this->getValue('name');
        }
        return $default;
    }

    // Anwendung z.B. $autor->getName('(unbekannt)');
    public function getName(string $default = ''): string
    {
        $value = $this->getValueWithFallback ($name, '');
        return (null === $value) ? $default : $value;
    }

[gharlan]

Ich glaube, ich würde es in Yorm lösen.

Bin nur noch nicht ganz sicher, wie am besten. Also ob getValue für nicht gesetzte Keys einfach null liefern sollte. Wäre hier wahrscheinlich praktisch. Andererseits finde ich immer gut, wenn Tippfehler per Exception frühzeitig auffallen. Also wenn man sich im Columnname bei getValue($colum) vertippt. Aber beim obigen Fall, frischer Datensatz, sind die Keys nun mal noch nicht gesetzt. Da es den Fall nun mal gibt, vielleicht hier doch besser null liefern für unbekannt Keys, auch wenn Tippfehler dann nicht so leicht auffallen.
Oder zwei Methoden, getValue und getValueOrNull oder so. Aber wenn man dann überall die Fallback-Methode nutzt, könnte auch getValue einfach so arbeiten.

(Andererseits frage ich mich aber auch, wieso du überhaupt für einen frischen Datensatz getName() aufrufst.)

[christophboecker]

(Andererseits frage ich mich aber auch, wieso du überhaupt für einen frischen Datensatz getName() aufrufst.)

Das war nur ein Beispiel; mir ist sowas tatsächlich mal passiert, weil ich annahm, dass der Default-Wert des zugehörigen Formularfeldes ausgegeben würde. Das ist tendenziell unsinnig, klar. Aber es kann ja tatsächlich passieren und ist dann mit 99,9% Wahrscheinlichkeit ein Programmierfehler, der in einem einem ordenlichen Woops enden sollte. Dann weiß man nämlich fix wo das Problem ist.

Tatsächlich sieht es übrigens so aus:

$autor = Author::create();
$name = $autor->getValue('name');
dump($name);

Aus dem Kontext "neues-Addon" geht es mir eher um die Frage, ob eine Methode wie getName immer einen String liefern sollte (also null abfangen und in '' umwandeln) oder ob die Rückgabe null einen Mehrwert hat bzw. als potentiell hilfreich gesehen wird ist. Ist ne Frage, die hier nur @alxndr-w beantworten kann.

[alxndr-w]

Ich würde da gerne noch das Ergebnis in https://github.com/yakamara/yform/pull/1520 abwarten. Vom Bauchgefühl her würde ich gerne kein neues Verhalten des Datasets nur in diesem Addon etablieren.

Insb., wenn es ein eher theoretischer Fall ist.