Beim Lesen dieser SO-Frage scheint es verpönt zu sein, Ausnahmen für die Überprüfung von Benutzereingaben auszulösen.

Aber wer sollte diese Daten validieren? In meinen Anwendungen werden alle Überprüfungen in der Geschäftsschicht durchgeführt, da nur die Klasse selbst wirklich weiß, welche Werte für jede ihrer Eigenschaften gültig sind. Wenn ich die Regeln für die Validierung einer Eigenschaft in den Controller kopieren würde, könnten sich die Validierungsregeln ändern, und jetzt gibt es zwei Stellen, an denen die Änderung vorgenommen werden sollte.

Ist meine Prämisse, dass die Validierung auf der Business-Ebene durchgeführt werden sollte, falsch?

Was ich mache

Mein Code endet normalerweise so:

<?php
class Person
{
  private $name;
  private $age;

  public function setName($n) {
    $n = trim($n);
    if (mb_strlen($n) == 0) {
      throw new ValidationException("Name cannot be empty");
    }
    $this->name = $n;
  }

  public function setAge($a) {
    if (!is_int($a)) {
      if (!ctype_digit(trim($a))) {
        throw new ValidationException("Age $a is not valid");
      }
      $a = (int)$a;
    }
    if ($a < 0 || $a > 150) {
      throw new ValidationException("Age $a is out of bounds");
    }
    $this->age = $a;
  }

  // other getters, setters and methods
}

In der Steuerung übergebe ich einfach die Eingabedaten an das Modell und fange ausgelöste Ausnahmen ab, um dem Benutzer die Fehler anzuzeigen:

<?php
$person = new Person();
$errors = array();

// global try for all exceptions other than ValidationException
try {

  // validation and process (if everything ok)
  try {
    $person->setAge($_POST['age']);
  } catch (ValidationException $e) {
    $errors['age'] = $e->getMessage();
  }

  try {
    $person->setName($_POST['name']);
  } catch (ValidationException $e) {
    $errors['name'] = $e->getMessage();
  }

  ...
} catch (Exception $e) {
  // log the error, send 500 internal server error to the client
  // and finish the request
}

if (count($errors) == 0) {
  // process
} else {
  showErrorsToUser($errors);
}

Ist das eine schlechte Methode?

Alternative Methode

Sollte ich vielleicht Methoden für isValidAge($a)diese Rückgabe true / false erstellen und sie dann vom Controller aufrufen?

<?php
class Person
{
  private $name;
  private $age;

  public function setName($n) {
    $n = trim($n);
    if ($this->isValidName($n)) {
      $this->name = $n;
    } else {
      throw new Exception("Invalid name");
    }
  }

  public function setAge($a) {
    if ($this->isValidAge($a)) {
      $this->age = $a;
    } else {
      throw new Exception("Invalid age");
    }
  }

  public function isValidName($n) {
    $n = trim($n);
    if (mb_strlen($n) == 0) {
      return false;
    }
    return true;
  }

  public function isValidAge($a) {
    if (!is_int($a)) {
      if (!ctype_digit(trim($a))) {
        return false;
      }
      $a = (int)$a;
    }
    if ($a < 0 || $a > 150) {
      return false;
    }
    return true;
  }

  // other getters, setters and methods
}

Und der Controller wird im Grunde der gleiche sein, nur anstatt zu versuchen / fangen gibt es jetzt if / else:

<?php
$person = new Person();
$errors = array();
if ($person->isValidAge($age)) {
  $person->setAge($age);
} catch (Exception $e) {
  $errors['age'] = "Invalid age";
}

if ($person->isValidName($name)) {
  $person->setName($name);
} catch (Exception $e) {
  $errors['name'] = "Invalid name";
}

...

if (count($errors) == 0) {
  // process
} else {
  showErrorsToUser($errors);
}

Also was soll ich tun?

Ich bin ziemlich zufrieden mit meiner ursprünglichen Methode, und meine Kollegen, denen ich sie im Allgemeinen gezeigt habe, haben sie gemocht. Sollte ich trotzdem auf die alternative Methode umsteigen? Oder mache ich das furchtbar falsch und sollte nach einem anderen Weg suchen?

Der Ansatz, den ich in der Vergangenheit verwendet habe, besteht darin, alle Validierungsklassen dedizierten Validierungsklassen zuzuordnen.

Sie können diese Validierungsklassen dann zur frühen Eingabevalidierung in Ihre Präsentationsschicht einfügen. Und nichts hindert Ihre Modellklassen daran, dieselben Klassen zu verwenden, um die Datenintegrität zu erzwingen.

Nach diesem Ansatz können Sie Validierungsfehler je nach Ebene, in der sie auftreten, unterschiedlich behandeln:

• Wenn die Überprüfung der Datenintegrität im Modell fehlschlägt, lösen Sie eine Ausnahme aus.
• Wenn die Überprüfung der Benutzereingaben in der Präsentationsebene fehlschlägt, zeigen Sie einen hilfreichen Tipp an und verzögern Sie die Übertragung des Werts auf Ihr Modell.

Ich bin ziemlich zufrieden mit meiner ursprünglichen Methode, und meine Kollegen, denen ich sie im Allgemeinen gezeigt habe, haben sie gemocht. Sollte ich trotzdem auf die alternative Methode umsteigen? Oder mache ich das furchtbar falsch und sollte nach einem anderen Weg suchen?

Wenn Sie und Ihre Kollegen damit zufrieden sind, sehe ich keinen dringenden Änderungsbedarf.

Das einzige, was aus pragmatischer Sicht fraglich ist, ist, dass Sie eher werfen Exceptionals etwas Spezifischeres. Das Problem ist, dass Sie beim Abfangen Exceptionmöglicherweise Ausnahmen abfangen , die nichts mit der Validierung von Benutzereingaben zu tun haben.

Jetzt gibt es viele Leute, die Dinge sagen wie "Ausnahmen sollten nur für außergewöhnliche Dinge verwendet werden, und XYZ ist nicht außergewöhnlich". (Zum Beispiel die Antwort von @ dann1111 ... wo er Benutzerfehler als "völlig normal" bezeichnet.)

Meine Antwort darauf ist, dass es kein objektives Kriterium für die Entscheidung gibt, ob etwas ("XY Z") außergewöhnlich ist oder nicht. Es ist eine subjektive Maßnahme. (Die Tatsache , dass einiger Programmbedarf für Fehler in Benutzereingaben überprüfen nicht den Auftritt Fehler „normal“ machen. In der Tat, „normal“ ist weitgehend sinnlos von einem objektiven Standpunkt aus .)

In diesem Mantra steckt ein Körnchen Wahrheit. In einigen Sprachen (oder genauer gesagt in einigen Sprachimplementierungen ) ist das Erstellen, Werfen und / oder Abfangen von Ausnahmen erheblich teurer als einfache Bedingungen. Wenn Sie es jedoch aus dieser Perspektive betrachten, müssen Sie die Kosten für das Erstellen / Werfen / Fangen mit den Kosten der zusätzlichen Tests vergleichen, die Sie möglicherweise durchführen müssen, wenn Sie die Verwendung von Ausnahmen vermeiden. Und die "Gleichung" muss die Wahrscheinlichkeit berücksichtigen, dass die Ausnahme ausgelöst werden muss.

Das andere Argument gegen Ausnahmen ist, dass sie das Verständnis von Code erschweren können . Aber die Kehrseite ist , dass , wenn sie in geeigneter Weise verwendet werden, können sie Code machen leichter zu verstehen.

Kurz gesagt - die Entscheidung, Ausnahmen zu verwenden oder nicht zu verwenden, sollte nach Abwägung der Verdienste getroffen werden ... und NICHT auf der Grundlage eines simplen Dogmas.

Meiner Meinung nach ist es nützlich, zwischen Anwendungsfehlern und Benutzerfehlern zu unterscheiden und nur Ausnahmen für die ersteren zu verwenden.

• Ausnahmen sollen Dinge abdecken, die verhindern, dass Ihr Programm ordnungsgemäß ausgeführt wird .

  Es handelt sich um unerwartete Ereignisse, die Sie daran hindern, fortzufahren, und ihr Design spiegelt dies wider: Sie unterbrechen die normale Ausführung und springen an einen Ort, an dem Fehler behandelt werden können.

• Benutzerfehler wie ungültige Eingaben sind (aus Sicht Ihres Programms) völlig normal und sollten von Ihrer Anwendung nicht als unerwartet behandelt werden .

  Wenn der Benutzer den falschen Wert eingibt und Sie eine Fehlermeldung anzeigen, ist Ihr Programm "fehlgeschlagen" oder ist in irgendeiner Weise ein Fehler aufgetreten? Nein. Ihre Bewerbung war erfolgreich. Bei einer bestimmten Art von Eingabe wurde in dieser Situation die richtige Ausgabe erstellt.

  Die Behandlung von Benutzerfehlern sollte, da sie Teil der normalen Ausführung ist, Teil Ihres normalen Programmablaufs sein und nicht durch Herausspringen mit einer Ausnahme behandelt werden.

Natürlich ist es möglich, Ausnahmen für andere Zwecke als den beabsichtigten Zweck zu verwenden, aber dies verwirrt das Paradigma und riskiert falsches Verhalten, wenn diese Fehler auftreten.

Ihr ursprünglicher Code ist problematisch:

• Der Aufrufer der setAge()Methode muss viel zu viel über die interne Fehlerbehandlung der Methode wissen: Der Aufrufer muss wissen, dass eine Ausnahme ausgelöst wird, wenn das Alter ungültig ist, und dass keine anderen Ausnahmen innerhalb der Methode ausgelöst werden können . Diese Annahme könnte später aufgehoben werden, wenn Sie zusätzliche Funktionen hinzufügen setAge().
• Wenn der Anrufer keine Ausnahmen abfängt, wird die ungültige Altersausnahme später auf eine andere, höchstwahrscheinlich undurchsichtige Weise behandelt. Oder sogar einen unbehandelten Ausnahme-Absturz verursachen. Kein gutes Verhalten für die Eingabe ungültiger Daten.

Der alternative Code hat auch Probleme:

• Eine zusätzliche, möglicherweise unnötige Methode isValidAge()wurde eingeführt.
• Jetzt muss die setAge()Methode davon ausgehen, dass der Anrufer isValidAge()das Alter bereits überprüft hat (eine schreckliche Annahme) oder erneut validiert. Wenn das Alter erneut überprüft wird, muss setAge() immer noch eine Art Fehlerbehandlung bereitgestellt werden, und Sie sind wieder auf dem ersten Platz.

Vorgeschlagenes Design

• Machen Sie die setAge()Rückkehr bei Erfolg wahr und bei Misserfolg falsch.

• Überprüfen Sie den Rückgabewert von setAge()und informieren Sie den Benutzer, falls dies fehlgeschlagen ist, dass das Alter ungültig war, nicht mit einer Ausnahme, sondern mit einer normalen Funktion, die dem Benutzer einen Fehler anzeigt.

Aus meiner Sicht (ich bin ein Java-Typ) ist es absolut gültig, wie Sie es auf die erste Weise implementiert haben.

Es ist gültig, dass ein Objekt eine Ausnahme auslöst, wenn einige Voraussetzungen nicht erfüllt sind (z. B. leere Zeichenfolge). In Java ist das Konzept der überprüften Ausnahmen für einen solchen Zweck vorgesehen - Ausnahmen, die in der Signatur deklariert werden müssen, um wahrscheinlich ausgelöst zu werden, und der Aufrufer muss diese explizit abfangen. Im Gegensatz dazu können ungeprüfte Ausnahmen (auch bekannt als RuntimeExceptions) jederzeit auftreten, ohne dass eine catch-Klausel im Code definiert werden muss. Während die ersten für wiederherstellbare Fälle verwendet werden (z. B. falsche Benutzereingaben, Dateiname existiert nicht), werden die letzteren für Fälle verwendet, gegen die der Benutzer / Programmierer nichts tun kann (z. B. zu wenig Speicher).

Sie sollten jedoch, wie bereits von @Stephen C erwähnt, Ihre eigenen Ausnahmen definieren und speziell diejenigen abfangen, um andere nicht unbeabsichtigt abzufangen.

Eine andere Möglichkeit wäre jedoch die Verwendung von Datenübertragungsobjekten, die einfach Datencontainer ohne Logik sind. Sie übergeben dann ein solches DTO zur Validierung an einen Validator oder das Modellobjekt selbst und nehmen nur bei Erfolg die Aktualisierungen im Modellobjekt vor. Dieser Ansatz wird häufig verwendet, wenn Präsentationslogik und Anwendungslogik getrennte Ebenen sind (Präsentation ist eine Webseite, Anwendung ein Webservice). Auf diese Weise werden sie physisch getrennt. Wenn Sie jedoch beide auf einer Ebene haben (wie in Ihrem Beispiel), müssen Sie sicherstellen, dass es keine Problemumgehung gibt, um einen Wert ohne Validierung festzulegen.

Mit meinem Haskell-Hut sind beide Ansätze falsch.

Was konzeptionell passiert, ist, dass Sie zuerst eine Reihe von Bytes haben und nach dem Parsen und Validieren eine Person erstellen können.

Die Person hat bestimmte Invarianten, wie die Vorwahl eines Namens und eines Alters.

Die Möglichkeit, eine Person zu repräsentieren, die nur einen Namen hat, aber kein Alter, möchten Sie unbedingt vermeiden, denn das schafft Vollständigkeit. Strenge Invarianten bedeuten, dass Sie beispielsweise später nicht mehr nach dem Vorhandensein eines Alters suchen müssen.

In meiner Welt wird Person atomar mithilfe eines einzelnen Konstruktors oder einer einzelnen Funktion erstellt. Dieser Konstruktor oder diese Funktion kann erneut die Gültigkeit der Parameter überprüfen, es sollte jedoch keine halbe Person erstellt werden.

Leider machen Java, PHP und andere OO-Sprachen die richtige Option ziemlich ausführlich. In geeigneten Java-APIs werden häufig Builder-Objekte verwendet. In einer solchen API würde das Erstellen einer Person ungefähr so ​​aussehen:

Person p = new Person.Builder().setName(name).setAge(age).build();

oder umso ausführlicher:

Person.Builder builder = new Person.Builder();
builder.setName(name);
builder.setAge(age);
Person p = builder.build();
// Person object must have name and age here

In diesen Fällen ist es unabhängig davon, wo Ausnahmen ausgelöst werden oder wo eine Validierung stattfindet, unmöglich, eine ungültige Personeninstanz zu empfangen.

In Laienwörtern:

Der erste Ansatz ist der richtige.

Der zweite Ansatz geht davon aus, dass diese Geschäftsklassen nur von diesen Controllern aufgerufen werden und dass sie niemals aus einem anderen Kontext aufgerufen werden.

Geschäftsklassen müssen jedes Mal eine Ausnahme auslösen, wenn eine Geschäftsregel verletzt wird.

Der Controller oder die Präsentationsschicht müssen entscheiden, ob sie ausgelöst werden oder ob sie eigene Validierungen durchführen, um zu verhindern, dass Ausnahmen auftreten.

Denken Sie daran: Ihre Klassen werden möglicherweise in unterschiedlichen Kontexten und von unterschiedlichen Integratoren verwendet. Sie müssen also klug genug sein, um Ausnahmen für schlechte Eingaben auszulösen.