Carlos Ble

Carlos Ble

I am a professional software developer, I solve problems.

I also teach and mentor developers to build better software.

Developing software since 2001.

Can I help you?

  • Do you need high quality tailor-made software?
  • Need training on TDD, clean code or refactoring?
  • Is it risky? do you need advise?
  • May I pair with you to write better code?

Events

Upcoming public training courses:

  1. [Online - en Español] 25 y 26 de Junio
    Test Doubles con JavaScript - Online
  2. [in English] July 7, 8 & 9
    TDD (open to the public) - Tenerife, Canary Islands
  3. [en Español] 14 y 15 Julio
    TDD (en abierto) - Gran Canaria, Canary Islands
  4. [in English] October 13, 14 & 15
    TDD (open to the public) - London, UK
  5. [en Español] 29, 30 y 31 de Octubre.
    TDD (en abierto) - Madrid, Spain

Conferences:

  1. I'll be at SocratesUK 2014
  2. I'll be at the London Test Gathering Workshops.

Un solo valor de retorno

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:

  1. var isNotValidData = !InputValidator.ValidateUserData(
  2. policyHolder.Person).success;
  3. if (isNotValidData)
  4. {
  5. response.message = Strings.T("Los datos son incorrectos.");
  6. return response;
  7. }

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.

 

Enjoyed reading this post?
Subscribe to the RSS feed and have all new posts delivered straight to you.
  • eliud

    Hola, al parecer usar heredar de ApplicationException no es muy recomendado, se menciona en la misma página.

    http://msdn.microsoft.com/en-us/library/system.applicationexception.aspx

  • Iñaki Elcoro

    Lo que comenta eliud sobre ApplicationException es cierto, es una costumbre heredada de la versión 1.1 de .NET donde existía la creencía de que para excepciones de “aplicación” era mejor heredar de esta última en lugar de hacerlo de System.Exception.

    Por otro lado aunque mayoritariamente estoy de acuerdo contigo, en el caso de las bases de datos (donde efectivamente excedemos los límites de nuestro sistema) sí tenemos la posibilidad de verificar si una operación ha ido bien.

    Todas las operaciones de modificación contra la base de datos devuelven el número de registros afectados por la operación. Asimismo muchos ORM también lo hacen. Por tanto es posible conocer, y en mi opinión recomendable, si la operación se ha ejecutado correctamente o no.

    Eso sí, sin lugar a dudas estas operaciones devolverían bool.

    Otra opción que también hemos utilizado mucho es cuando es necesario conocer si el registro existe antes de insertarlo o modificarlo (en entornos multiusuario por ejemplo) donde devolvemos una enumeración que se comprueba en un controlador de operaciones de datos.

  • http://carlosble.com Carlos Ble

    No sabia que ApplicationException ya estaba desaconsejada. Nosotros le sacamos partido. Si la excepcion que viene es de este tipo sabemos que no hay que registrarla en los logs como excepcion inesperada, asi evitamos hacer registro de la misma excepcion varias veces.

    En cuanto a lo que comentas Iñaki, eso es casarte con la BD. Aunque tu BD te devuelva el numero de registros, yo haria que el metodo fuese void y que lanzase una excepcion si los registros son cero y deberian ser mas. Me aseguro que los componentes de terceros no afectan la solidez de mis repositorios. A ese nivel registras lo que quieras y de ahi hacia las capas superiores te manejas con lo que tu controlas sin que importe lo que viene por debajo.

    Gracias por el feedback

  • http://geeks.ms/blogs/devnettips Vicenç García

    Hola Carlos,

    estoy de acuerdo contigo pero no se si me cuadra mucho eso de que una excepción pueda ser esperada. Si es esperada, no es excepción, ¿no?

    Ya que pones el código, se me ocurre otra observación, de la que Iñaki y yo ya hemos discutido (amistosamente, claro) alguna vez y es el uso del var. Entiendo lo que defiende Iñaki de poder utilizar var cuando estas haciendo un new, es decir, tienes el tipo en la misma línea de código, pero en el código que pones, de un vistazo no se qué tipo es la variable isNotValidData. Vale que lo puedo llegar a saber del nombre de la variable, pero no siempre se ponen buenos nombres en las variables. Yo, en este caso, si que pondría bool.

    Un saludo!

  • http://geeks.ms/blogs/lruiz Luis Ruiz Pavon

    Buenas,

    Yo creo que el principal problema radica en no entender que es un DTO, y un DTO no se puede pasar entre capas de nuestra aplicacion, para eso esta nuestro dominio no? y en caso de metodos del Is… Does… yo soy d eutilizar un booleano.

    Ahora, si lo que necesitamos es no exponer nuestro dominio, que es lo normal, podemos usar DTO en el caso de comunicarnos con otras aplicaciones y ViewModels para solo mostrar aquella informacion necesaria en las vistas de nuestra aplicacion.

    Sobre las excepciones, muchas veces nuestro problema es intentar capturar todos los errores para darle la mayor informacion al usuario. Al usuario no le interesa sino hay conexion con la base de datos o si tu sql ha fallado porque un campo de la tabla no existe, esta informacion dependiendo en que aplicaciones puede ser hasta peligrosa, con decirle que se ha producido un error y que lo vuelva a intentar tiene mas que suficiente. Debemos capturar las excepciones siempre que tengamos que hacer algo con ella, existe un antipatron que es el de capturar, loggear y relanzar, esto muchas veces lleva a perder informacion del error y luego dificulta su resolucion en entornos de produccion. Yo siempre capturo la excepcion en el punto superior, si son excepciones que no esperas, por ejemplo en ASP.NET tienes el global.asax.

    Respecto al uso de var, que yo era reticente a su uso, Hadi Hariri me respondio muy bien en su dia, abriendome los ojos:

    “Tu problema ahí no es var sino como estás nombrando las cosas, algo que resumí hace unos días aquí:”

    http://devlicio.us/blogs/hadi_hariri/archive/2009/11/20/var-improves-readability.aspx

    Os recomiendo leer :)

    Muy buen post Carlos!

    Un saludo

  • http://carlosble.com Carlos Ble

    Gracias por el feedback. Luis ha respondido muy bien en mi opinion. Pero me gustaria añadir que en Python, Ruby, Perl, Javascript, etc, no se definen los tipos de las variables y se puede programar :-)
    Es nuestra obligacion hacer el código suficientemente semántico.
    Además nos apoyamos en el IDE. Si quiero saber de qué tipo es esa variable en cualquier punto, solo tengo que poner el puntero encima y Visual Studio me lo dice. Ya no leemos el código en papel asi que cuento con el IDE como parte inseparable de mi trabajo cuando se trata de mantener el código.

    En cuanto a excepciones, una cosa es que esten controladas y otra cosa que no. Pero uno puede estar perfectamente preparado para situaciones excepcionales :-)

    El capítulo de excepciones de Code Complete (McConnell) lo explica muy bien.

    Logicamente un pedazo de código sacado de contexto siempre se presta a muchas interpretaciones.