Hay un mal que se esta extendiendo por la aplicación en las últimas semanas. Es el ResponseDTO. Es un objeto plano, tiene un campo “success” de tipo boolean y un campo “message” de tipo string. Debía llamarse Response pero colisionaba, así que se quedó con un mal nombre. Si le hubiesemos llamado ControllerResponse seguramente no se hubiese usado en las otras capas de la aplicación porque hubiese dado más pistas a quienes lo fuesen usar, de que su lugar es la capa controller. Sin embargo se ha colado en muchas capas de backend y no es una buena práctica:
Los metodos y funciones deben devolver un solo resultado y en ocasiones, ninguno (void). ResponseDTO se puede considerar “un solo resultado” cuando los campos “success” y “message” se necesitan el uno al otro, tal que no tiene sentido el objeto sin los dos campos. Es decir cuando el objeto es lo más atómico que la operación puede devolver. ResponseDTO se ideo para enviar a cliente (js) respuesta a determinados comandos que solicita al servidor pero no debería usarse en las otras capas de la aplicacion.
¿Dónde lo usamos mal? Tenemos métodos con un nombre interrogativo que denotan retorno boolean y sin embargo devuelven uno de estos ResponseDTO:
- IsExistingUser
- DoesPolicyExists
- IsValidInput
Para cualquiera que lea la API, no es intuitivo. ¿Para qué sirve el “message” del objeto cuando el método “IsExistingUser” retorna? Para nada.
En la validación, puede tener sentido este objeto response dependiendo del contexto: Si el error de validación puede considerarse una circunstancia excepcional, entonces lo mejor es lanzar una excepción que alguien de nivel superior recogerá. Por tanto, sobra el objeto response ya que la excepción llevará el mensaje de validación particular. Si un problema de validación no se considera una circunstancia excepcional, entonces puede estar bien devolver el response con su success puesto a false y su mensaje concreto.
Más sitios donde lo usamos mal: Métodos que disparan acciones hacia fuera de nuestro sistema
- SendEmail
- SaveUser
Los métodos que provocan acciones que salen de los límites de nuestro código (acceso a BD, a un servidor de correo, etc) no pueden garantizar la correcta consumación de la acción. Por tanto, jugar a devolver boolean para exito o fracaso es engañarnos.
Es preferible un método void que funciona sin hacer ruido cuando todo va bien y que lanza excepciones cuando se encuentra con una situación inesperada. Por ejemplo, el método que solicita el envio de un email, puede defenderse de excepciones que provoque la libreria de SMTP y traducirlas en excepciones de nuestro dominio, tales como EmailSendFailure (una excepción nuestra que hereda de ApplicationException).
¿Y es tan grave que usemos el ResponseDTO? Sí que lo es porque el código que consume esos métodos se convierte en spaguetti:
var isNotValidData = !InputValidator.ValidateUserData(
policyHolder.Person).success;
if (isNotValidData)
{
response.message = Strings.T("Los datos son incorrectos.");
return response;
}
Del método “ValidateUserData” sólo nos interesa el campo “success”, el mensaje lo está generando el llamador. No sólo es raro de leer sino que es un coñazo a la hora de escribir tests de interacción. Cuando queremos que en un test de colaboración, este método devuelva falso, tenemos que especificar que devuelva un objeto con un campo a falso. Si fuera boolean, no tendriamos que especificar nada porque el valor por defecto sería false. Esto va al hilo del posts de los tests de interacción limpios.
En el caso de métodos de acción como el envio de email, es incluso peor porque quien consume el método pensará que con un resultado verdadero el email ha llegado a su destino, como si eso lo pudiesemos asegurar de alguna manera.
Para saber qué debe devolver un método, este debe tener una única responsabilidad y debemos saber cual es. Y la mejor manera de saberlo, para mi siempre es escribir el test antes que el código.