Problem mit Rückgabewert

[Yadgar]

Hi(gh)!

Folgende Funktion zur Überprüfung von Formulardaten:

  function check()  
  {  
    $ok = false;  
    if (!$_POST['Marke'])  
      echo "<p>Bitte wählen Sie eine Marke aus!</p>";  
    else if (!$_POST['Modell'])  
      echo "<p>Bitte geben Sie ein Modell ein!</p>";  
    else if ($_POST['Markteinfuehrung'] != "" && $_POST['EndeProduktion'] != "" && ((!is_numeric($_POST['Markteinfuehrung']) || !is_numeric($_POST['EndeProduktion']) || $_POST['Markteinfuehrung']<1934 || $_POST['EndeProduktion']<1934 )))  
      echo "<p>Bitte geben Sie nur Jahreszahlen ab 1934 ein!</p>";  
    else if ($_POST['Markteinfuehrung'] > date("Y") || $_POST['EndeProduktion'] > date("Y"))  
      echo "<p>Mindestens eine von Ihnen eingegebene Jahreszahl liegt in der Zukunft. Bitte geben sie Jahreszahlen von 1934 bis ".date("Y")." ein!</p>";  
    else if ($_POST['Markteinfuehrung'] > $_POST['EndeProduktion'])  
      echo "<p>Das Markteinführungsjahr muss früher als das Jahr des Produktionsendes sein. Bitte korrigieren Sie Ihre Eingabe!</p>";  
    else if (($_POST['Neupreis'] = "" && $_POST['Waehrung'] != "") || ($_POST['Neupreis'] != "" && $_POST['Waehrung'] == ""))  
      echo '<p>Bitte geben Sie Neupreis <span class="b">und</span> Währung ein!</p>';  
    else if ($_POST['Neupreis'] != "" && $_POST['Waehrung'] != "")  
    {  
      if (!is_numeric($_POST['Neupreis']) || $_POST['Neupreis'] <= 0)  
        echo '<p>Bitte geben Sie für den Neupreis einen Zahlenwert größer als 0 ein!</p>';  
    }  
    else if ($_POST['Breite'] != "")  
    {  
      if (!is_numeric($_POST['Breite']) || $_POST['Breite'] <= 0 || $_POST['Breite'] >= 500)  
        echo '<p>Bitte geben Sie für die Breite einen Zahlenwert zwischen 0 und 500 ein!</p>';  
    }  
    else if ($_POST['Hoehe'] != "")  
    {  
      if (!is_numeric($_POST['Hoehe']) || $_POST['Hoehe'] <= 0 || $_POST['Hoehe'] >= 500)  
        echo '<p>Bitte geben Sie für die Höhe einen Zahlenwert zwischen 0 und 500 ein!</p>';  
    }  
    else if ($_POST['Tiefe'] != "")  
    {  
      if (!is_numeric($_POST['Tiefe']) || $_POST['Tiefe'] <= 0 || $_POST['Tiefe'] >= 500)  
        echo '<p>Bitte geben Sie für die Tiefe einen Zahlenwert zwischen 0 und 500 ein!</p>';  
    }  
    else if ($_POST['Gewicht'] != "")  
    {  
      if (!is_numeric($_POST['Gewicht']) || $_POST['Gewicht'] <= 0 || $_POST['Gewicht'] >= 1000)  
        echo '<p>Bitte geben Sie für das Gewicht einen Zahlenwert zwischen 0 und 1000 ein!</p>';  
    }  
    else if ($_POST['Registerspeicher'] != "")  
    {  
      if (!is_numeric($_POST['Registerspeicher']) || $_POST['Registerspeicher'] < 0 || $_POST['Registerspeicher'] > 255)  
        echo '<p>Bitte geben Sie für die Anzahl der Registerspeicher einen Zahlenwert von 0 bis 255 ein!</p>';  
    }  
    else if ($_POST['Akkordspeicher'] != "")  
    {  
      if (!is_numeric($_POST['Akkordspeicher']) || $_POST['Akkordspeicher'] < 0 || $_POST['Akkordspeicher'] > 65535)  
        echo '<p>Bitte geben Sie für die Anzahl der Akkordspeicher einen Zahlenwert von 0 bis 65535 ein!</p>';  
    }  
    else if ($_POST['Schweller'] != "")  
    {  
      if (!is_numeric($_POST['Schweller']) || $_POST['Schweller'] < 0 || $_POST['Schweller'] > 9)  
        echo '<p>Bitte gegen Sie für die Anzahl der Schweller einen Zahlenwert von 0 bis 9 ein!</p>';  
    }  
    else if ($_POST['Gesamtleistung'] != "")  
    {  
      if (!is_numeric($_POST['Gesamtleistung']) || $_POST['Gesamtleistung'] <= 0 || $_POST['Gesamtleistung'] > 65535)  
      echo '<p>Bitte geben Sie für die Gesamtleistung einen Zahlenwert zwischen 0 und 65536 ein!</p>';  
    }  
    else $ok = true;  
  
    if ($ok == true)  
      return true;  
    else  
      return false;  
  }

soll true zurückgeben, falls keines der Fehler-Kriterien erfüllt ist.

Es wird aber, selbst wenn keine der Fehlerbedingungen erfüllt ist, immer false zurückgegeben! Warum?

Bis bald im Khyberspace!

Yadgar

[Felix Riesterer]

Lieber Yadgar,

Dein Code ist im höchsten Maße unleserlich. Das will ich überhaupt nicht genauer studieren.

Aber: Was machst Du, wenn nicht alle $_POST-Schlüssel im Request enthalten sind? Wenn z.B. 'Marke' nicht übermittelt wird? Dann wird PHP mit einer Fehlermeldung abbrehen.

Mein Vorschlag: Strukturiere Deinen Code besser, dann findet sich auch der Fehler schneller.

Liebe Grüße,

Felix Riesterer.

[Encoder]

Gibt es vielleicht unerwartetes Verhalten mit true und false?
Debugausgabe ins else $ok = true einbauen und schauen ob das Script dort ankommt.

else $ok = true;

Ich würde statt dieser Zeile gleich true zurückgeben:
if ...
else if ...
else return true;

return false; // wenn nicht return true

Wenn ich den Code so lese fällt mir die Diskussion ein, warum manche Programmierer Variablen umkopieren und ob man das tun sollte oder nicht.

  
if (!is_numeric($_POST['Breite']) || $_POST['Breite'] <= 0 || $_POST['Breite'] >= 500)  
        echo '...';  
  
if (!is_numeric($Breite) || $Breite <= 0 || $Breite >= 500)  
        echo '...';  

Die umkopierte Variante wäre für mich die schönere und einfacher zu überschauende. Kürzer, weniger Sonderzeichen die man beim drüberlesen ausblenden muss. Viel einfacher zu schreiben, weniger Fehlermöglichkeiten...
Ganz verstanden habe ich noch nie, warum die Frage nach dem umkopieren immer wieder kommt. Beim einmaligen Zugriff ist es schon nachvollziehbar zu überlegen ob man sich das umkopieren antun will.
Hier frage ich mich, warum tut man es nicht?

[Camping_RIDER]

Aloha ;)

Ich würde statt dieser Zeile gleich true zurückgeben:
if ...
else if ...
else return true;

return false; // wenn nicht return true

Oder auch gleich:

  
If (Fehler1) {  
    echo 'Fehler1'; return false;  
}  
  
If (Fehler2) {  
    Echo 'Fehler2'; return false;  
}  
  
...  
  
return true;  

Das erfüllt denselben Zweck (aufgrund der else-if's wird auch beim ursprünglichen Code maximal eine Fehlermeldung ausgegeben), vermeidet aber diese unglaublich riesige if-else-if-Struktur und ist dadurch lesbarer und weniger Fehleranfällig. Sollen alle Fehler ausgegeben werden würde ich sowas empfehlen:

  
$ok = true;  
  
If (Fehler1) {  
    echo 'Fehler1'; $ok = false;  
}  
  
If (Fehler2) {  
    Echo 'Fehler2'; $ok = false;  
}  
  
...  
  
return $ok;  

Auch übersichtlicher als die ursprüngliche Variante ;)

Grüße,

RIDER

[dedlfix]

Tach!

Sollen alle Fehler ausgegeben werden würde ich sowas empfehlen:

$ok = true;

If (Fehler1) {
    echo 'Fehler1'; $ok = false;
}

If (Fehler2) {
    Echo 'Fehler2'; $ok = false;
}

...

return $ok;

  
Besser fände ich:  
  
~~~php
$errors = array();  
  
if (fehler1)  
  $errors[] = 'fehlermeldung 1';  
  
if (fehler2)  
  $errors[] = 'fehlermeldung 2';

Wir sind an dieser Stelle noch beim Verarbeiten der Werte. Eine Ausgabe sollte nicht Teil dieses Schritts sein. Am Ende der Prüfung kann man auf leeres oder gefülltes Array prüfen, um festzustellen, ob Fehler auftraten. Für die Ausgabe kann man über das Array iterieren. Dann kann man auch noch entscheiden, ob diese auf den Bildschirm oder ins Log oder beides oder sonstwohin erfolgen soll. Damit trennt man die Teilaufgaben und baut sich keine unnötigen Abhängigkeiten ein. Oder verbaut sich vielleicht durch die zu frühe Ausgabe, HTTP-Header senden zu können, die erst später im Programmablauf erstellt werden.

dedlfix.

[Camping_RIDER]

Aloha ;)

Besser fände ich:

$errors = array();

if (fehler1)
  $errors[] = 'fehlermeldung 1';

if (fehler2)
  $errors[] = 'fehlermeldung 2';

  
Chapeau! Und der Rückgabewert der Funktion check wäre dann das $errors-Array. Sollte noch ein Boolean $ok benötigt werden, kann dieser einfach in der übergeordneten Funktion erhalten werden:  
  
~~~php
$errors = check();  
$ok = empty($errors);

Bitte beachten: empty() arbeitet nur mit Variablen, ein Aufruf wie $ok = empty(check()); erzeugt einen ParseError...

Grüße,

RIDER

[dedlfix]

Tach!

Bitte beachten: empty() arbeitet nur mit Variablen, ein Aufruf wie $ok = empty(check()); erzeugt einen ParseError...

Da hängt mal wieder die deutsche Übersetzung hinterher. Ab PHP 5.5 kann empty() auch Ausdrücke.

dedlfix.

[Gunnar Bittersmann]

@@dedlfix:

nuqneH

Besser fände ich:

$errors = array();

if (fehler1)
  $errors[] = 'fehlermeldung 1';

if (fehler2)
  $errors[] = 'fehlermeldung 2';

  
Und was willst du mit dem Array von „Fehlermeldungen“ dann anfangen? Etwa als Block ausgeben?  
  
[Nei-en!!](https://forum.selfhtml.org/?t=218816&m=1507833)  
  
Man braucht die „Fehlerbehandlung“ für jedes Eingabefeld einzeln.  
  
Qapla'

-- 
„Talente finden Lösungen, Genies entdecken Probleme.“ (Hans Krailsheimer)

[Camping_RIDER]

Aloha ;)

Und was willst du mit dem Array von „Fehlermeldungen“ dann anfangen? Etwa als Block ausgeben?

Hehe, da haben sich unsere Antworten wohl zeitlich überschnitten, siehe meine Antwort auf deinen ursprünglichen Beitrag.

Grüße,

RIDER

[dedlfix]

Tach!

Und was willst du mit dem Array von „Fehlermeldungen“ dann anfangen? Etwa als Block ausgeben?

Das Array muss etwas verbessert werde, so dass man daraus auch noch die Feldnamen entnehmen kann.

$errors[$feldname][] = 'text'; // pro Feldname ein Array mit den Texten, können ja jeweils mehrere sein.

Nei-en!!
Man braucht die „Fehlerbehandlung“ für jedes Eingabefeld einzeln.

Dann kann man das gezielt pro Feld ausgeben.

dedlfix.

[Matthias Apsel]

Om nah hoo pez nyeetz, dedlfix!

Dann kann man das gezielt pro Feld ausgeben.

Und an das Feld ranschreiben.

Matthias

[Matthias Apsel]

Om nah hoo pez nyeetz, dedlfix!

$errors = array();

if (fehler1)
  $errors[] = 'fehlermeldung 1';

if (fehler2)
  $errors[] = 'fehlermeldung 2';

  
Ich habe an solchen Stellen immer array\_push($fehler,'Fehlermeldung'); verwendet. Ist das besser/schlechter/anders/gleich gut?  
  
  
Matthias

-- 
Der Unterschied zwischen Java und JavaScript ist größer als der zwischen [Vase und Vaseline](http://selfhtml.apsel-mv.de/java-javascript/index.php?buchstabe=V#vase).  
![](http://www.billiger-im-urlaub.de/kreis_sw.gif)  

[dedlfix]

Tach!

Ich habe an solchen Stellen immer array_push($fehler,'Fehlermeldung'); verwendet. Ist das besser/schlechter/anders/gleich gut?

Es ist ein Funktionsaufruf mitsamt dem dazugehörigen Overhead. Der geht bei einfachen Anwendungen zwar im Grundrauschen unter, ...

array_push() spielt erst dann einen Vorteil aus, wenn man mehrere Werte aus einmal anhängen möchte. Und dann gibts da noch einen kleinen Unterschied, der in der zweiten Note im Handbuch erläutert wird.

dedlfix.

[Matthias Apsel]

Om nah hoo pez nyeetz, dedlfix!

Wir sind an dieser Stelle noch beim Verarbeiten der Werte. Eine Ausgabe sollte nicht Teil dieses Schritts sein. Am Ende der Prüfung kann man auf leeres oder gefülltes Array prüfen, um festzustellen, ob Fehler auftraten. Für die Ausgabe kann man über das Array iterieren. Dann kann man auch noch entscheiden, ob diese auf den Bildschirm oder ins Log oder beides oder sonstwohin erfolgen soll. Damit trennt man die Teilaufgaben und baut sich keine unnötigen Abhängigkeiten ein. Oder verbaut sich vielleicht durch die zu frühe Ausgabe, HTTP-Header senden zu können, die erst später im Programmablauf erstellt werden.

Von mir ein fachlich hilfreich.

Da sieht man mal wieder wie viele Gedanken man sich _vor_ dem Programmieren eigentlich machen kann (lies: sollte)

Matthias

[dedlfix]

Tach!

Gibt es vielleicht unerwartetes Verhalten mit true und false?

Die Frage ist eher, welche Werte wirklich verarbeitet werden. Allein aus dem Code kann man vielleicht irgendwo eine unlogische Konstellation finden, die immer true oder false ergibt. Wenn es allerdings abhängig von den verarbeiteten Werten ist, nützt der Code allein nicht viel.

Debugausgabe ins else $ok = true einbauen und schauen ob das Script dort ankommt.

Von oben nach unten durcharbeiten wäre mein Vorschlag. Mit var_dump(...) die Werte anschauen und die Ergebnisse der jeweiligen Ausdrücke. Und das sowohl bei Teil-Ausdrücken als auch bei der gesamten Bedingung für die ifs. Das ist die "Drecksarbeit", die ein Programmierer immer machen muss, bevor er drüber nachdenkt, was denn geändert werden muss. Ohne das Wissen, welcher Wert an welcher Stelle das Problem verusacht, ist in der Regel keine gescheite Antwort möglich. Es ist auch nicht besonders nett, einem Forum den Code reinzukippen und nur "da läuft was schief" zu sagen. Besser ist: "Ich hab an der und der Stelle den und den Wert und bekomme das Ergebnis X, erwarte aber Y. Der Ursprung der Werte ist dort und sie werden auf dem Weg bis zur Problemstelle hier und da verarbeitet." Diese Formulierung zu finden ist meist schon die halbe, oft aber auch die ganze Miete für die Fehlerfindung.

Wenn ich den Code so lese fällt mir die Diskussion ein, warum manche Programmierer Variablen umkopieren und ob man das tun sollte oder nicht.
Die umkopierte Variante wäre für mich die schönere und einfacher zu überschauende. Kürzer, weniger Sonderzeichen die man beim drüberlesen ausblenden muss. Viel einfacher zu schreiben, weniger Fehlermöglichkeiten...

Das sind Argumente dafür. Die meisten machen das aber ohne Nachzudenken und oftmals verwenden sie den umkopierten Wert einmal. Das ist dann nur noch Mehrarbeit.

Ganz verstanden habe ich noch nie, warum die Frage nach dem umkopieren immer wieder kommt. Beim einmaligen Zugriff ist es schon nachvollziehbar zu überlegen ob man sich das umkopieren antun will.
Hier frage ich mich, warum tut man es nicht?

Vermutlich wieder wegen zu wenig Nachdenken. Eine sinnvolle Alternative statt einfachem Umkopieren wäre eine Aufteilung der Prüfung. Der erste Teil nimmt die Eingabewerte entgegen und erzeugt daraus ein Objekt oder Array. Dabei wird schon auf das Vorhandensein der Werte geprüft und ansonsten ein sinnvoller Default-Wert vergeben (beispielsweise Leerstring oder 0 oder null). Gemäß dem DRY-Prinzip (don't repeat yourself) macht man nicht für jeden Parameter händisch die ganze Prozedur sondern schreibt sich eine kleine Funktion dafür. Auch das Durchlaufen für alle Werte kann man auf ein Array mit den Namen und eine foreach-Schleife recht übersichtlich gestalten. Das entstandene Objekt/Array übergibt man dann der Funktion, die die fachliche Prüfung vornimmt.

Es ist nicht generell verkehrt, Werte von einer Form in eine andere umzukopieren. Es sollte nur mit Sinn und Verstand erfolgen.

dedlfix.

[Stefanie M.]

Hallo,

wenn alle Dein Post-Variablen einen Inhalt haben, kommst Du nie in den else-Zweig, der Dein ok auf true setzt. Warum machst Du am Ende eigentlich nicht einfach "return $ok"?

Gruß
Stefanie

[Stefanie M.]

PS: Wenn Du einmal mit einem Debugger durchgesteppt wärst, wäre Dir das übrigens sofort aufgefallen.

[Gunnar Bittersmann]

@@Yadgar:

nuqneH

Folgende Funktion zur Überprüfung von Formulardaten:

… kannst du dir nochmal überlegen. Ganz von vorne.

Es ist alles andere als nutzerfreundlich, wenn „Fehlermeldungen“* irgendwo im Nirgendwo (am Anfang oder Ende des Formulars) stehen, der Nutzer sie lesen und sich selbst das (die) entsprechende(n) Eingabefeld(er) im Formular raussuchen muss.

„Fehlermeldungen“ gehören an Ort und Stelle – genau dorthin, wo der „Fehler“ aufgetreten ist. Also direkt über oder unter das (die) jeweilige(n) Eingabefeld(er), welche(s) zusätzlich noch hervorgehoben werden sollte, damit der Nutzer erkennt, wo noch Eingaben nötig sind.

Oder man bietet dem Nutzer beim Affenformular nur noch die Felder an, die noch einer Eingabe bedürfen (mit Option, alles nochmal anzusehen).

Qapla'

* in Anführungszeichen, weil dem Nutzer nicht kommuniziert werden sollte, dass er/sie der/die Dumme ist und einen „Fehler“ gemacht hat

[Camping_RIDER]

Aloha ;)

Es ist alles andere als nutzerfreundlich, wenn „Fehlermeldungen“* irgendwo im Nirgendwo (am Anfang oder Ende des Formulars) stehen, der Nutzer sie lesen und sich selbst das (die) entsprechende(n) Eingabefeld(er) im Formular raussuchen muss.

Um dedlfix` Beispiel aufzugreifen - das liese sich in etwa so realisieren:

$errors = array();  
  
if (fehler1)  
    $errors['formularfeld1'] = 'fehlermeldung 1';  
  
if (fehler2)  
    $errors['formularfeld2'] = 'fehlermeldung 2';

Dann ist es sowohl möglich, die Fehlermeldungen in einen globalen Fehlerlog einzuschleusen (wenn das gewünscht und/oder sinnvoll ist) als auch über die Formularkennung der einzelnen Fehlermeldungen bei der erneuten Formularausgabe den entsprechenden Formularfeldern die jeweilige "Fehlermeldung" (oder besser: "Benachrichtigung über ausstehende Korrekturen" :P) voranzustellen. Auch eine relativ einfache Realisierung des angesprochenen "Affenformulars" ist damit möglich (z.B. über isset($errors['formularfeldxy']) )

Noch eine Anmerkung: Bei der erneuten Ausgabe des Formulars ist es nicht nur sinnvoll, die Fehlermeldungen direkt zuzuordnen, sondern auch die Formularfelder mit den schon eingegebenen Daten ausgefüllt auszuliefern. Nichts nervt mich als user mehr, als alle Daten noch einmal neu eingeben zu müssen nur weil ich mich in einem einzigen Feld vertippt habe... Ganz besonders schick wirds, wenn die Formularfelder, die einen Fehler ergeben haben, auch noch rot hinterlegt werden...

Grüße,

RIDER

[Gunnar Bittersmann]

@@Camping_RIDER:

nuqneH

sondern auch die Formularfelder mit den schon eingegebenen Daten ausgefüllt auszuliefern.

Das dies bei einem Affenformular geschieht, davon bin ich ausgegangen.

Ganz besonders schick wirds, wenn die Formularfelder, die einen Fehler ergeben haben, auch noch rot hinterlegt werden...

Sag ich doch: „… zusätzlich noch hervorgehoben werden sollte, damit der Nutzer erkennt, wo noch Eingaben nötig sind.“

Ob nun roter Hintergrund oder roter Rahmen …

Qapla'

[Gunnar Bittersmann]

@@Camping_RIDER:

nuqneH

Um dedlfix` Beispiel aufzugreifen - das liese sich in etwa so realisieren:

$errors = array();

if (fehler1)
    $errors['formularfeld1'] = 'fehlermeldung 1';

if (fehler2)
    $errors['formularfeld2'] = 'fehlermeldung 2';

  
Wenn man etwas dynamisch in einem Array sammeln möchte, dann die Fehler\*, nicht die Meldungen.  
  
Die für alle möglicherweise auftretenden Fehlerfälle benötigten statischen Meldungen dürfen gerne in einem anderen Array stehen.  
  
Qapla'  
  
\* da wir uns jetzt auf technischer Ebene befinden, ohne Anführungszeichen

-- 
„Talente finden Lösungen, Genies entdecken Probleme.“ (Hans Krailsheimer)

[dedlfix]

Tach!

Wenn man etwas dynamisch in einem Array sammeln möchte, dann die Fehler*, nicht die Meldungen.

Ja, das wäre dann der perfekte Weg. Wenn das aber unter YAGNI fällt, weil ich beispielsweise keine Mehrsprachigkeit brauche, dann geh ich lieber den einfacheren Weg mit dem fertige Text. Getreu dem Motto: Man muß die Dinge so richtig wie möglich machen. Aber nicht richtiger. (Ja, ich weiß, im Original war es "einfach".)

dedlfix.

[Camping_RIDER]

Aloha ;)

Man muß die Dinge so richtig wie möglich machen. Aber nicht richtiger. (Ja, ich weiß, im Original war es "einfach".)

Sehe ich auch so. Alle bisherigen Anmerkungen hatten einen tatsächlichen universellen Benefit. Ob es sinnvoller ist, Fehler zu speichern oder Fehlermeldungen ist doch wirklich hochgradig vom spezifischen Problem abhängig. Zumal es auch eine Interpretationssache ist. Denn wenn ich die Fehlermeldungen in einem Array speichere, habe ich damit auch den Fehler gespeichert - nämlich über isset($errors['fehler']) bzw. den Array der Keys von $errors. Wenn das Programm gar kein Fehler-Logging betreibt oder betreiben soll ist die Speicherung der Meldung von viel größerer Bedeutung als die Speicherung des Fehlers.

Bisher ging es vor allem darum, das gegebene Problem effizienter und sinnvoller zu lösen. Jetzt über Zusatzfeatures zu diskutieren, die erstens nicht notwendig sind und zweitens das gegebene Problem nicht vereinfachen, ist nicht unbedingt zielführend ;)

Grüße,

RIDER