Czytając to pytanie SO , wydaje się, że odrzucanie wyjątków w celu sprawdzania poprawności danych wejściowych użytkownika jest niezadowolone.

Ale kto powinien zweryfikować te dane? W moich aplikacjach wszystkie walidacje są wykonywane w warstwie biznesowej, ponieważ tylko sama klasa naprawdę wie, które wartości są prawidłowe dla każdej z jej właściwości. Gdybym miał skopiować zasady sprawdzania poprawności właściwości do kontrolera, możliwe jest, że reguły sprawdzania poprawności ulegną zmianie i teraz są dwa miejsca, w których należy dokonać modyfikacji.

Czy moje założenie, że walidacja powinna odbywać się w warstwie biznesowej, jest błędne?

Co robię

Więc mój kod zwykle kończy się tak:

<?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
}

W kontrolerze po prostu przekazuję dane wejściowe do modelu i wychwytuję zgłoszone wyjątki, aby pokazać użytkownikowi błąd:

<?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);
}

Czy to zła metodologia?

Metoda alternatywna

Czy powinienem utworzyć metody isValidAge($a)zwracania wartości prawda / fałsz, a następnie wywołać je ze sterownika?

<?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
}

Kontroler będzie w zasadzie taki sam, tylko zamiast try / catch są teraz 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);
}

Więc co powinienem zrobić?

Jestem bardzo zadowolony z mojej oryginalnej metody i podobają mi się moi koledzy, którym to pokazałem. Mimo to, czy powinienem przejść na alternatywną metodę? A może robię to okropnie źle i powinienem szukać innej drogi?

Podejście, które stosowałem w przeszłości, polega na umieszczeniu wszystkich dedykowanych klas logiki walidacji.

Następnie można wstrzyknąć te klasy sprawdzania poprawności do warstwy prezentacji w celu wczesnego sprawdzenia poprawności danych wejściowych. Nic nie stoi na przeszkodzie, aby klasy Model korzystały z tych samych klas do egzekwowania integralności danych.

Postępując zgodnie z tym podejściem, możesz następnie traktować Błędy Walidacji w różny sposób, w zależności od warstwy, w której się znajdują:

• Jeśli sprawdzanie integralności danych nie powiedzie się w modelu, należy zgłosić wyjątek.
• Jeśli sprawdzanie poprawności danych wejściowych przez użytkownika nie powiedzie się w warstwie prezentacji, wyświetl pomocną wskazówkę i opóźnij przekazanie wartości do modelu.

Jestem bardzo zadowolony z mojej oryginalnej metody i podobają mi się moi koledzy, którym to pokazałem. Mimo to, czy powinienem przejść na alternatywną metodę? A może robię to okropnie źle i powinienem szukać innej drogi?

Jeśli ty i twoi koledzy jesteście z tego zadowoleni, nie widzę pilnej potrzeby zmiany.

Jedyną rzeczą, która jest wątpliwa z pragmatycznego punktu widzenia, jest to, że rzucasz, Exceptiona nie coś bardziej konkretnego. Problem polega na tym, że jeśli złapiesz Exception, możesz złapać wyjątki, które nie mają nic wspólnego z weryfikacją danych wejściowych użytkownika.

Teraz jest wielu ludzi, którzy mówią, że „wyjątki należy stosować tylko do wyjątkowych rzeczy, a XYZ nie jest wyjątkowy”. (Na przykład @ dann1111's Answer ... gdzie opisuje błędy użytkownika jako „całkowicie normalne”).

Odpowiadam na to, że nie ma obiektywnego kryterium decydującego o tym, czy coś („XY Z”) jest wyjątkowe, czy nie. Jest to miara subiektywna . (Fakt, że jakikolwiek program musi sprawdzić błędy w danych wejściowych użytkownika, nie powoduje, że błędy wystąpienia są „normalne”. W rzeczywistości „normalne” jest w dużej mierze bez znaczenia z obiektywnego punktu widzenia.)

W tej mantrze jest ziarno prawdy. W niektórych językach (a dokładniej w niektórych implementacjach językowych ) tworzenie wyjątków, rzucanie i / lub łapanie jest znacznie droższe niż proste warunki warunkowe. Ale jeśli spojrzysz na to z tej perspektywy, musisz porównać koszty tworzenia / rzucania / łapania z kosztami dodatkowych testów, które możesz potrzebować, jeśli unikniesz wyjątków. „Równanie” musi brać pod uwagę prawdopodobieństwo, że wyjątek musi zostać zgłoszony.

Innym argumentem przeciwko wyjątkom jest to, że mogą utrudniać zrozumienie kodu. Ale drugą stroną jest to, że gdy są właściwie używane, mogą ułatwić zrozumienie kodu .

W skrócie - decyzja o zastosowaniu lub nie wykorzystaniu wyjątków powinna być podjęta po rozważeniu zalet ... a NIE na podstawie jakiegoś uproszczonego dogmatu.

Moim zdaniem użyteczne jest rozróżnienie między błędami aplikacji i błędami użytkownika i stosowanie tylko tych wyjątków.

• Wyjątki mają obejmować rzeczy, które uniemożliwiają prawidłowe działanie programu .

  Są to nieoczekiwane zdarzenia, które uniemożliwiają ci kontynuowanie, a ich konstrukcja odzwierciedla to: przerywają normalne wykonanie i przeskakują w miejsce, które umożliwia obsługę błędów.

• Błędy użytkownika, takie jak nieprawidłowe dane wejściowe, są całkowicie normalne (z punktu widzenia programu) i nie powinny być traktowane jako nieoczekiwane przez aplikację .

  Jeśli użytkownik wpisze nieprawidłową wartość i wyświetli się komunikat o błędzie, to czy Twój program „zawiódł” lub wystąpił błąd w jakikolwiek sposób? Nie. Twoja aplikacja zakończyła się powodzeniem - biorąc pod uwagę pewien rodzaj danych wejściowych, w tej sytuacji wygenerowała poprawny wynik.

  Obsługa błędów użytkowników, ponieważ jest to część normalnego wykonywania, powinna być częścią normalnego przepływu programu, a nie obsługiwana przez wyskakiwanie z wyjątkiem.

Oczywiście możliwe jest stosowanie wyjątków do celów innych niż zamierzone, ale robi to mylący paradygmat i ryzykuje nieprawidłowe zachowanie w przypadku wystąpienia tych błędów.

Twój oryginalny kod jest problematyczny:

• Osoba wywołująca setAge()metodę musi wiedzieć zbyt dużo o wewnętrznej obsłudze błędów metody: osoba dzwoniąca musi wiedzieć, że wyjątek jest zgłaszany, gdy wiek jest nieważny, i że żadne inne wyjątki nie mogą być zgłaszane w ramach metody . To założenie może zostać później złamane, jeśli dodasz w nim dodatkową funkcjonalność setAge().
• Jeśli osoba dzwoniąca nie wyłapie wyjątków, niepoprawny wyjątek wieku zostanie później obsłużony w inny, najprawdopodobniej nieprzejrzysty sposób. Lub nawet spowodować awarię nieobsługiwanego wyjątku. Niewłaściwe zachowanie w przypadku wprowadzania nieprawidłowych danych.

Kod alternatywny ma również problemy:

• Wprowadzono dodatkową, być może niepotrzebną metodę isValidAge().
• Teraz setAge()metoda musi zakładać, że dzwoniący już sprawdził isValidAge()(straszne założenie) lub ponownie zweryfikować wiek. Jeśli ponownie potwierdzi wiek, setAge() nadal musi zapewnić obsługę błędów, a ty znów powracasz do normy.

Sugerowany projekt

• Spraw, aby setAge()zwrot był prawdziwy w przypadku sukcesu, a fałszywy w przypadku niepowodzenia.

• Sprawdź wartość zwracaną, setAge()a jeśli to się nie powiedzie, poinformuj użytkownika, że ​​wiek był nieprawidłowy, nie z wyjątkiem, ale z normalną funkcją, która wyświetla użytkownikowi błąd.

Z mojego punktu widzenia (jestem facetem Java) jest całkowicie poprawne, jak zaimplementowałeś go w pierwszy sposób.

Prawidłowe jest, że obiekt zgłasza wyjątek, gdy pewne warunki wstępne nie są spełnione (np. Łańcuch pusty). W Javie koncepcja sprawdzonych wyjątków jest przeznaczona do takiego celu - wyjątki, które muszą zostać zadeklarowane w podpisie, aby zostały rzucone, a osoba dzwoniąca musi je wyraźnie złapać. Natomiast niesprawdzone wyjątki (znane również jako RuntimeExceptions) mogą wystąpić w dowolnym momencie bez potrzeby definiowania klauzuli catch w kodzie. Podczas gdy pierwsze są używane w przypadkach, które można odzyskać (np. Nieprawidłowe dane wprowadzone przez użytkownika, nazwa pliku nie istnieje), drugie są używane w przypadkach, w których użytkownik / programista nie może nic zrobić (np. Brak pamięci).

Powinieneś jednak, jak już wspomniano w @Stephen C, zdefiniować własne wyjątki i złapać te wyjątki, aby nie przypadkowo złapać innych.

Innym sposobem byłoby jednak użycie obiektów przesyłania danych, które są po prostu kontenerami danych bez logiki. Następnie przekazujesz takie DTO walidatorowi lub samemu obiektowi modelowemu w celu sprawdzenia poprawności i tylko w przypadku powodzenia dokonaj aktualizacji w obiekcie modelowym. Takie podejście jest często stosowane, gdy logika prezentacji i logika aplikacji są oddzielnymi poziomami (prezentacja to strona internetowa, aplikacja to usługa internetowa). W ten sposób są one fizycznie rozdzielone, ale jeśli masz oba na jednym poziomie (jak w twoim przykładzie), musisz upewnić się, że nie będzie obejścia, aby ustawić wartość bez sprawdzania poprawności.

Z założonym kapeluszem Haskell oba podejścia są błędne.

To, co dzieje się koncepcyjnie, polega na tym, że najpierw masz kilka bajtów, a po analizie i sprawdzeniu poprawności możesz następnie zbudować Osobę.

Osoba ma pewne niezmienniki, takie jak precense imienia i wieku.

Możliwość reprezentowania Osoby, która ma tylko imię, ale nie ma wieku, jest czymś, czego chcesz uniknąć za wszelką cenę, ponieważ to tworzy komplementarność. Surowe niezmienniki oznaczają, że nie musisz na przykład sprawdzać obecności wieku później.

Więc w moim świecie Osoba jest tworzona atomowo za pomocą jednego konstruktora lub funkcji. Ten konstruktor lub funkcja może ponownie sprawdzić poprawność parametrów, ale nie należy budować żadnej osoby.

Niestety, Java, PHP i inne języki OO sprawiają, że poprawna opcja jest dość szczegółowa. W odpowiednich interfejsach API Java często używane są obiekty konstruktora. W takim interfejsie API utworzenie osoby wyglądałoby mniej więcej tak:

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

lub bardziej szczegółowy:

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

W takich przypadkach, bez względu na to, gdzie zgłaszane są wyjątki lub gdzie odbywa się sprawdzanie poprawności, nie można uzyskać wystąpienia Osoby, które jest nieprawidłowe.

Słowami laika:

Pierwsze podejście jest prawidłowe.

Drugie podejście zakłada, że ​​te klasy biznesowe będą wywoływane tylko przez tych kontrolerów i że nigdy nie będą wywoływane z innego kontekstu.

Klasy biznesowe muszą zgłaszać wyjątek za każdym razem, gdy naruszona zostanie reguła biznesowa.

Kontroler lub warstwa prezentacji musi zdecydować, czy je wyrzuca, czy też wykonuje własne walidacje, aby zapobiec wystąpieniu wyjątków.

Pamiętaj: twoje klasy będą potencjalnie wykorzystywane w różnych kontekstach i przez różnych integratorów. Muszą więc być wystarczająco inteligentni, aby rzucać wyjątki od złych danych wejściowych.