Al leer esta pregunta SO , parece que lanzar excepciones para validar la entrada del usuario está mal visto.
Pero, ¿quién debería validar estos datos? En mis aplicaciones, todas las validaciones se realizan en la capa empresarial, porque solo la clase en sí misma sabe qué valores son válidos para cada una de sus propiedades. Si tuviera que copiar las reglas para validar una propiedad al controlador, es posible que las reglas de validación cambien y ahora hay dos lugares donde se debe realizar la modificación.
¿Mi premisa de que la validación debe hacerse en la capa empresarial es incorrecta?
Lo que hago
Entonces mi código generalmente termina así:
<?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
}
En el controlador, simplemente paso los datos de entrada al modelo y capturo las excepciones lanzadas para mostrar los errores al usuario:
<?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);
}
¿Es esta una mala metodología?
Metodo alternativo
¿Debería crear métodos para isValidAge($a)
ese retorno verdadero / falso y luego llamarlos desde el controlador?
<?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
}
Y el controlador será básicamente el mismo, solo que en lugar de try / catch hay ahora 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);
}
¿Entonces qué debo hacer?
Estoy bastante contento con mi método original, y a mis colegas a quienes se lo mostré en general les ha gustado. A pesar de esto, ¿debería cambiar al método alternativo? ¿O estoy haciendo esto terriblemente mal y debería buscar otra forma?
fuente
ValidationException
y otras excepcionesIValidateResults
.Respuestas:
El enfoque que he usado en el pasado es poner todas las clases de validación dedicadas a la lógica de validación.
Luego puede inyectar estas clases de validación en su capa de presentación para una validación de entrada temprana. Y nada impide que sus clases Modelo utilicen las mismas clases para imponer la Integridad de los datos.
Siguiendo este enfoque, puede tratar los errores de validación de manera diferente dependiendo de en qué capa ocurran:
fuente
PersonValidator
con toda la lógica para validar los diferentes atributos de aPerson
, y laPerson
clase que depende de estoPersonValidator
, ¿verdad? ¿Cuál es la ventaja que ofrece su propuesta sobre el método alternativo que sugerí en la pregunta? Solo veo la capacidad de inyectar diferentes clases de Validación para aPerson
, pero no puedo pensar en ningún caso en el que esto sea necesario.Si usted y sus colegas están contentos con esto, no veo la necesidad urgente de cambiar.
Lo único que es cuestionable desde una perspectiva pragmática es que estás tirando en
Exception
lugar de algo más específico. El problema es que si detectaException
, puede terminar detectando excepciones que no tienen nada que ver con la validación de la entrada del usuario.Ahora hay muchas personas que dicen cosas como "las excepciones solo deben usarse para cosas excepcionales, y XYZ no es excepcional". (Por ejemplo, la respuesta de @ dann1111 ... donde etiqueta los errores del usuario como "perfectamente normales").
Mi respuesta a eso es que no hay un criterio objetivo para decidir si algo ("XY Z") es excepcional o no. Es una medida subjetiva . (El hecho de que cualquier programa necesite verificar errores en la entrada del usuario no hace que los errores de ocurrencia sean "normales". De hecho, "normal" carece de sentido desde un punto de vista objetivo).
Hay un grano de verdad en ese mantra. En algunos idiomas (o más exactamente, algunas implementaciones de lenguaje ) la creación, lanzamiento y / o captura de excepciones es significativamente más costosa que los condicionales simples. Pero si lo mira desde esa perspectiva, debe comparar el costo de crear / lanzar / atrapar con el costo de las pruebas adicionales que podría necesitar realizar si evitaba usar excepciones. Y la "ecuación" tiene que tener en cuenta la probabilidad de que deba lanzarse la excepción.
El otro argumento contra las excepciones es que pueden dificultar la comprensión del código. Pero la otra cara es que cuando se usan adecuadamente, pueden hacer que el código sea más fácil de entender.
En resumen, la decisión de usar o no usar excepciones debe hacerse después de sopesar los méritos ... y NO sobre la base de algún dogma simplista.
fuente
Exception
arrojado / atrapado. Realmente lanzo alguna subclase propiaException
, y el código de los setters generalmente no hace nada que pueda lanzar otra excepción.En mi opinión, es útil distinguir entre los errores de la aplicación y los errores del usuario , y solo usar excepciones para los primeros.
Las excepciones están destinadas a cubrir cosas que impiden que su programa se ejecute correctamente .
Son sucesos inesperados que le impiden continuar, y su diseño refleja esto: interrumpen la ejecución normal y saltan a un lugar que permite el manejo de errores.
Los errores del usuario, como la entrada no válida, son perfectamente normales (desde la perspectiva de su programa) y su aplicación no debe tratarlos como inesperados .
Si el usuario ingresa el valor incorrecto y usted muestra un mensaje de error, ¿su programa "falló" o tuvo algún error? No. Su aplicación fue exitosa: dado un cierto tipo de entrada, produjo la salida correcta en esa situación.
El manejo de los errores del usuario, debido a que es parte de la ejecución normal, debería ser parte del flujo normal de su programa, en lugar de manejarse saltando con una excepción.
Por supuesto, es posible usar excepciones para otro propósito que no sea el previsto, pero hacerlo confunde el paradigma y se arriesga a comportamientos incorrectos cuando se producen esos errores.
Su código original es problemático:
setAge()
método debe saber demasiado sobre el manejo de errores internos del método: la persona que llama necesita saber que se produce una excepción cuando la edad no es válida, y que no se pueden generar otras excepciones dentro del método . Esta suposición podría romperse más adelante si agrega una funcionalidad adicional dentrosetAge()
.El código alternativo también tiene problemas:
isValidAge()
ha introducido un método adicional, posiblemente innecesario .setAge()
método tiene que suponer que la persona que llamó ya marcóisValidAge()
(una suposición terrible) o validar la edad nuevamente. Si vuelve a validar la antigüedad,setAge()
aún debe proporcionar algún tipo de manejo de errores, y volverá al punto de partida nuevamente.Diseño sugerido
Haga que el
setAge()
retorno sea verdadero en caso de éxito y falso en caso de fracaso.Verifique el valor de retorno
setAge()
y, si falla, informe al usuario que la edad no era válida, no con una excepción, sino con una función normal que muestra un error al usuario.fuente
setAge
a validar de nuevo, pero como la lógica es básicamente "si es válida, establezca la edad y la excepción de lanzamiento" no me lleva de vuelta al punto de partida."The entered age is out of bounds" == true
personas siempre deberían usar===
, por lo que este enfoque sería más problemático que el problema que intenta resolversetAge()
que haces en cualquier lugar, debes verificar que realmente funcionó. Lanzar excepciones significa que no debe preocuparse por recordar que todo salió bien. Tal como lo veo, tratar de establecer un valor no válido en un atributo / propiedad es algo excepcional y, por lo tanto, vale la pena lanzar elException
. Al modelo no debería importarle si obtiene su entrada de la base de datos o del usuario. Nunca debería recibir una entrada incorrecta, por lo que veo que es legítimo lanzar una excepción allí.Desde mi punto de vista (soy un chico de Java) es totalmente válido cómo lo implementó la primera vez.
Es válido que un objeto arroje una Excepción cuando no se cumplen algunas condiciones previas (por ejemplo, cadena vacía). En Java, el concepto de excepciones comprobadas está destinado a tal propósito: excepciones que deben declararse en la firma para que se puedan lanzar de manera propagable, y la persona que llama explícitamente necesita capturarlas. En contraste, las excepciones no verificadas (también conocidas como RuntimeExceptions) pueden ocurrir en cualquier momento sin la necesidad de definir una cláusula catch en el código. Mientras que los primeros se usan para casos recuperables (por ejemplo, entrada incorrecta del usuario, el nombre de archivo no existe), los segundos se usan para casos en los que el usuario / programador no puede hacer nada (por ejemplo, sin memoria).
Sin embargo, como ya mencionó @Stephen C, debe definir sus propias excepciones y capturar específicamente aquellas para no atrapar a otros sin querer.
Sin embargo, otra forma sería utilizar objetos de transferencia de datos que son simplemente contenedores de datos sin ninguna lógica. Luego transfiere dicho DTO a un validador o al Modelo-Objeto para su validación, y solo si tiene éxito, realice las actualizaciones en el Modelo-Objeto. Este enfoque se usa a menudo cuando la lógica de presentación y la lógica de aplicación son niveles separados (la presentación es una página web, la aplicación es un servicio web). De esta manera, se separan físicamente, pero si tiene ambos en un nivel (como en su ejemplo), debe asegurarse de que no habrá una solución alternativa para establecer un valor sin validación.
fuente
Con mi sombrero Haskell puesto, ambos enfoques están equivocados.
Lo que sucede conceptualmente es que primero tienes un montón de bytes, y después de analizar y validar, puedes construir una Persona.
La Persona tiene ciertos invariantes, como el precedencia de un nombre y una edad.
Ser capaz de representar a una Persona que solo tiene un nombre, pero que no tiene edad, es algo que desea evitar a toda costa, porque eso es lo que crea la totalidad. Los invariantes estrictos significan que no es necesario verificar la presencia de una edad posterior, por ejemplo.
Entonces, en mi mundo, Person se crea atómicamente usando un solo constructor o función. Ese constructor o función puede verificar nuevamente la validez de los parámetros, pero no se debe construir una media persona.
Desafortunadamente, Java, PHP y otros lenguajes OO hacen que la opción correcta sea bastante detallada. En las API de Java adecuadas, a menudo se usan los objetos del generador. En tal API, crear una persona se vería así:
o el más detallado:
En estos casos, no importa dónde se produzcan excepciones o donde se realice la validación, es imposible recibir una instancia de Persona que no sea válida.
fuente
En palabras simples:
El primer enfoque es el correcto.
El segundo enfoque supone que esas clases de negocios solo serán llamadas por esos controladores, y que nunca serán llamadas desde otro contexto.
Las clases de negocios deben lanzar una excepción cada vez que se infringe una regla de negocios.
El controlador o la capa de presentación deben decidir si los arroja o si hace sus propias validaciones para evitar que ocurran las excepciones.
Recuerde: sus clases serán potencialmente utilizadas en diferentes contextos y por diferentes integradores. Por lo tanto, deben ser lo suficientemente inteligentes como para lanzar excepciones a las entradas incorrectas.
fuente