Читая этот 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
}

В контроллере я просто передаю входные данные в модель и ловлю выданные исключения, чтобы показать пользователю ошибки:

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

Это плохая методология?

Альтернативный метод

Должен ли я создать методы для isValidAge($a)возврата true / false и затем вызвать их из контроллера?

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

И контроллер будет в основном таким же, просто вместо try / catch теперь есть 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);
}

И что я должен делать?

Я очень доволен своим оригинальным методом, и мои коллеги, которым я его показал, в целом понравились. Несмотря на это, я должен перейти на альтернативный метод? Или я делаю это ужасно неправильно, и я должен искать другой путь?

Подход, который я использовал в прошлом, заключается в размещении всей логики валидации в выделенных классах валидации.

Затем вы можете внедрить эти классы валидации в свой уровень представления для ранней проверки ввода. И ничто не мешает вашим модельным классам использовать те же классы для обеспечения целостности данных.

Следуя этому подходу, вы можете по-разному относиться к ошибкам проверки в зависимости от того, в каком слое они происходят:

• Если в модели не выполняется проверка целостности данных, генерируется исключение.
• Если проверка входных данных пользователя не пройдена на уровне представления, отобразите полезный совет и задержите отправку значения в вашу модель.

Я очень доволен своим оригинальным методом, и мои коллеги, которым я его показал, в целом понравились. Несмотря на это, я должен перейти на альтернативный метод? Или я делаю это ужасно неправильно, и я должен искать другой путь?

Если вы и ваши коллеги довольны этим, я не вижу необходимости менять ситуацию.

С прагматической точки зрения сомнительно только то, что вы бросаете, Exceptionа не что-то более конкретное. Проблема в том, что если вы поймаете Exception, вы можете в конечном итоге поймать исключения, которые не имеют ничего общего с проверкой пользовательского ввода.

Сейчас есть много людей, которые говорят что-то вроде «исключения должны использоваться только для исключительных вещей, и XYZ не исключение». (Например, ответ @ dann1111 ... где он помечает ошибки пользователя как «совершенно нормальные».)

Мой ответ на это заключается в том, что не существует объективного критерия для решения, является ли что-то ("XY Z") исключительным или нет. Это субъективная мера. (Тот факт, что любая программа должна проверять наличие ошибок при вводе пользователем, не делает ошибки возникновения «нормальными». Фактически, «нормальное» в значительной степени бессмысленно с объективной точки зрения.)

В этой мантре есть доля правды. В некоторых языках (или, точнее, в некоторых языковых реализациях ) создание, выброс и / или перехват исключений значительно дороже, чем простые условные выражения . Но если вы посмотрите на это с этой точки зрения, вам нужно сравнить стоимость create / throw / catch со стоимостью дополнительных тестов, которые вам, возможно, придется выполнить, если вы избегаете использования исключений. И «уравнение» должно учитывать вероятность того, что исключение должно быть выброшено.

Другой аргумент против исключений состоит в том, что они могут усложнить понимание кода. Но обратная сторона в том, что при правильном использовании они могут облегчить понимание кода .

Короче говоря, решение об использовании или неприменении исключений должно приниматься после взвешивания достоинств ... а НЕ на основе какой-то упрощенной догмы.

На мой взгляд, полезно различать ошибки приложений и пользовательских ошибок , и только исключения использования для первого.

• Исключения предназначены для охвата вещей, которые мешают вашей программе работать должным образом .

  Это непредвиденные события, которые мешают вам продолжить, и их дизайн отражает это: они нарушают нормальное выполнение и переходят в место, где допускается обработка ошибок.

• Ошибки пользователя, такие как неверный ввод, являются совершенно нормальными (с точки зрения вашей программы) и не должны рассматриваться вашим приложением как неожиданные .

  Если пользователь вводит неправильное значение, и вы выводите сообщение об ошибке, произошла ли ошибка вашей программы или возникла ошибка? Нет. Ваша заявка была успешной - с определенным видом ввода она выдает правильные результаты в этой ситуации.

  Обработка пользовательских ошибок, потому что это является частью нормального выполнения, должна быть частью вашего обычного программного потока, а не обрабатываться путем выпрыгивания с исключением.

Конечно, можно использовать исключения не по назначению, но это приводит к путанице и может привести к неправильному поведению при возникновении таких ошибок.

Ваш оригинальный код проблематичен:

• Вызывающий setAge()метод должен слишком много знать об обработке внутренних ошибок метода: вызывающий должен знать, что исключение выдается, когда недопустимый возраст, и что никакие другие исключения не могут быть сгенерированы в методе . Это предположение может быть нарушено позже, если вы добавите дополнительную функциональность в setAge().
• Если вызывающая сторона не перехватывает исключения, недопустимое возрастное исключение будет позже обработано другим, наиболее вероятным, непрозрачным способом. Или даже вызвать необработанное исключение аварии. Не хорошее поведение при вводе неверных данных.

Альтернативный код также имеет проблемы:

• Был isValidAge()введен дополнительный, возможно ненужный метод .
• Теперь setAge()метод должен предположить, что вызывающий абонент уже проверил isValidAge()(ужасное предположение) или подтвердить возраст еще раз. Если он снова проверяет возраст, setAge() все равно должен обеспечить некоторую обработку ошибок, и вы снова возвращаетесь к исходной точке.

Предлагаемый дизайн

• Верните setAge()true в случае успеха и false в случае неудачи.

• Проверьте возвращаемое значение setAge()и, если это не удалось, сообщите пользователю, что возраст был недействительным, не с исключением, но с нормальной функцией, которая отображает ошибку для пользователя.

С моей точки зрения (я парень из Java) совершенно верно, как вы реализовали это первым способом.

Действительно, объект генерирует исключение, когда некоторые предварительные условия не выполняются (например, пустая строка). В Java концепция проверенных исключений предназначена для такой цели - исключения, которые должны быть объявлены в сигнатуре, чтобы их можно было выбросить, и вызывающий объект должен явно их перехватить. Напротив, непроверенные исключения (или RuntimeExceptions) могут возникать в любое время без необходимости определять выражение catch в коде. В то время как первые используются для восстанавливаемых случаев (например, неправильный ввод пользователя, имя файла не существует), последние используются для случаев, с которыми пользователь / программист ничего не может сделать (например, нехватка памяти).

Тем не менее, как уже упоминалось @Stephen C, вы должны определить свои собственные исключения и отловить их, чтобы они не были непреднамеренно.

Однако другим способом было бы использовать объекты передачи данных, которые являются просто контейнерами данных без какой-либо логики. Затем вы передаете такой DTO валидатору или самому Model-Object для проверки и только в случае успеха вносите обновления в Model-Object. Этот подход часто используется, когда логика представления и логика приложения являются разделенными уровнями (представление - это веб-страница, приложение - веб-служба). Таким образом, они физически разделены, но если у вас есть оба на одном уровне (как в вашем примере), вы должны убедиться, что не будет никакого обходного пути для установки значения без проверки.

С моей шляпой на Haskell оба подхода неверны.

Концептуально происходит то, что у вас сначала есть куча байтов, а после анализа и проверки вы можете создать Person.

Человек имеет определенные инварианты, такие как прецедент имени и возраста.

Возможность представлять Человека, у которого есть только имя, но без возраста, - это то, чего вы хотите избежать любой ценой, потому что именно это создает целостность. Строгие инварианты означают, что вам не нужно, например, проверять наличие возраста позже.

Так что в моем мире Person создается атомарно с помощью одного конструктора или функции. Этот конструктор или функция может снова проверить правильность параметров, но не нужно создавать половину человека.

К сожалению, Java, PHP и другие ОО-языки делают правильную опцию довольно многословной. В правильных API Java часто используются объекты-строители. В таком API создание человека будет выглядеть примерно так:

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

или более многословный:

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

В этих случаях, независимо от того, где генерируются исключения или когда происходит проверка, невозможно получить экземпляр Person, который является недействительным.

По словам непрофессионала:

Первый подход является правильным.

Второй подход предполагает, что эти бизнес-классы будут вызываться только этими контроллерами и что они никогда не будут вызываться из другого контекста.

Бизнес-классы должны выдавать исключение каждый раз, когда нарушается бизнес-правило.

Контроллер или уровень представления должны решить, генерирует ли он их или выполняет свои собственные проверки для предотвращения возникновения исключений.

Помните: ваши классы потенциально будут использоваться в разных контекстах и ​​разными интеграторами. Поэтому они должны быть достаточно умными, чтобы создавать исключения для неверных входных данных.