if (factura.getEstado() == EstadoFactura.PAGA) {
// código
} else if (factura.getEstado() == EstadoFactura.ENTREGADA) {
// código
} else {
// otro
}
no puedo evitar pensar: "¡horrible!". Por que conozco los problemas que genera este tipo de código y sé que con otro diseño pueden evitarse.
La cuestión es explicar como mejorar el diseño. En estas situaciones, mis intentos de explicación son frases un tanto pedantes como: "este uso del 'if' es feo, por que las decisiones sobre que hacer según un 'estado', en objetos se pueden resolver utilizando polimorfismo".
Lamentablemente este esbozo de explicación no ayuda, y noto que en general produce la siguiente reacción:
"Todo muy lindo esto de objetos, mensajes y polimorfismo. Pero tengo en mi tabla FACTURA un campo ESTADO y esto refleja directamente eso. Además ¿Cual es la solución? ¿Usar el patrón State? ¿Hacer un montón de clases para algo que resuelvo en un 'if'? Dejemos la 'estética de objetos' para la teoría."
Una explicación técnica...
Voy a comenzar por el camino común de dejar en claro por que en objetos este tipo de "if" puede evitarse.
El siguiente ejemplo:
if (figura.getTipo() == Figura.RECTANGULO) {
area = figura.getBase() * figura.getAltura();
} else if (figura.getTipo() == Figura.TRIANGULO) {
area = figura.getBase() * figura.getAltura() / 2;
} else if (figura .... ya se hacen a la idea de como sigue
puede resolverse mediante clases que representen a los rectángulos, triángulos, etc.
Si todas estas clases son polimorficas con el mensaje "getArea()" entonces toda la seguidilla de "if" puede resolverse en una sola línea: figura.getArea().
Donde el receptor del mensaje es quien encapsula la decisión de como hacer las cosas.
Complicando el problema
Muchas veces es deseable que el algoritmo a ejecutar según el tipo de objeto este separado de los objetos.Por ejemplo si Figura posee un método "dibujar" es probable que el algoritmo de dibujo genere dependencias con un framework de dibujo, y quizás no quiero "atar" al modulo de figuras con un framework de dibujo en particular, o quizás el algoritmo de dibujo varíe según el contexto.
Para este tipo de casos las alternativas son usar Double Dispatch o bien algún tipo de interfaz entre distintos frameworks/implementaciones donde en este caso dibujar seria algo así como "dibujarSobre(unCanvas)" siendo "unCanvas" es un objeto que implementa esa interfaz común.
Pero volviendo al problema original (de la Factura y Estado), hay algunas cuestiones a tener en cuenta:
- En el caso de la factura el "if" se hace en base al estado y no a un "tipo de factura", eso significa que a diferencia del ejemplo de las figuras el estado puede variar una vez creada la factura.
- Crear un objeto que represente el estado y usar Double Dispatch no parece ser en este caso una buena solución. (El por que lo dejo como ejercicio)
¿Cuál es la solución a este problema?
Rta: Examinar mejor el dominio:
- ¿Tiene sentido hablar de "estado" de una factura?
- ¿Que representa la factura?
- ¿Realmente el cambio de "estado" representa el cambio de estado de una misma cosa o en realidad representa cosas distintas?
Examinando el dominio
NOTA: El ejemplo de Factura/Estado es ficticio, pero creo que captura muchos de los casos de negocio donde encontré código del estilo "if estado then ...".En "Stan's Shop" la gente agrega productos a su pedido, paga en la caja donde se le entrega una factura, finalmente en el mostrador de entregas un empleado prepara los productos y una vez que se los dio al cliente coloca un lindo sello de "ENTREGADO" en la factura (un esquema similar al que siguen todos los locales de comida rápida del micro-centro).
Los diseñadores del sistema pensaron que era bueno tener un objeto "factura" especificado por la siguiente clase:
Este diseño tiene varios problemas:
La solución a este problema es sencilla:
Con este diseño la separación de responsabilidades es clara, no hay forma que en el código modifique items en una factura, o de pedirle el número de factura a un CarritoDeCompras, por lo tanto no hay necesidad de verificar un "estado".
El caso de "ENTREGADA" lo voy a discutir a continuación por que me sirve para ejemplificar otro "patrón" común.
No quiero entrar en detalles de este dominio ficticio, pero quizás un "flag" no alcance. Es probable que quiera registrar quien realizo la entrega, a que hora, etc.
Por eso voy a hacer una simplificación: no me interesan esos detalles, solo quiero el equivalente al sello de "ENTREGADA".
Y dado que en este ejemplo yo pongo las reglas, voy a suponer que el operador en el mostrador de entregas tiene una pantalla que muestra las facturas pendientes de entrega, donde con un click puede cambiar el estado a "ENTREGADA".
Una forma de diseñar esto es pensar que uno tiene un "sistema" (modulo, o como deseen nombrar) que lleva el control de los pedidos, con dos "recipientes": uno para la facturas pedientes y otro para las entregadas. Entonces pasar de estado es mover la Factura de un recipiente a otro:
¿Pero el booleano no era mejor? No, este esquema tiene muchas ventajas más:
¿Y la UI? "Quisiera hacer una página web que muestre el listado de facturas entregadas y no entregadas, con getEstado() o el flag es mucho más fácil".
En este caso también es fácil por que puede resolverse de la siguiente manera:
¿Y la base de datos? "Tengo una tabla Factura que tiene la columna ENTREGADA"
No hay problema: las herramientas de mapeo O/R permiten hacer mapeos complejos de asociaciones para resolver el caso entregada/pendiente en el SistemaDePedidos, o si es necesario discriminar la subclase a mapear.
Creo que en la balanza de "bueno/malo" el código del estilo "if estado then codigo" tiene muchos problemas:
Mientras que las únicas ventajas que le veo son que es simple de implementar para ejemplos chicos y fácil de mapear a una tabla en la base de datos.
Por todas estas razones conviene evitar este tipo de "ifs" y en lo posible evitar el uso de Enums ya que fomentan este tipo de código.
Este diseño tiene varios problemas:
- Si la factura se ve como un comprobante de pago (al momento de pensar este ejemplo ficticio desconozco si en el negocio tiene otros usos), entonces la factura debe ser inmutable. Este hecho se refleja en que agregarItem y quitarItem generan error: solución problemática por que en todos los lugares donde quiera enviar estos mensajes tengo que tener en cuenta la posibilidad de error. Lo mismo ocurre con getNumero donde se debe chequear por null.
- La "mutabilidad" de factura genera otros problemas de implementación, pero prefiero dejar los detalles para otro post sobre "side effects".
- ¿Cómo sé cuando una factura es "valida"? Puedo chequear por getEstado() == PAGA o ENTREGADA, puedo chequear por getNumero() != null o agregar un nuevo método esValida (de intention revealing ni hablar). En la práctica encontré que se mezclan las tres formas dependiendo del programador, lo que genera algunos dolores de cabeza al momento de hacer un refactoring.
- ¿Tiene sentido tener un estado "ENTREGADA"? ¿El hecho de saber si los productos fueron entregados o no, no es responsabilidad de otra área de negocio? Este es un ejemplo ficticio y no aporta mucho escarbar en detalles de negocio, simplemente lo menciono por que en la mayoría de los casos los problemas de diseño muestran una falla en reconocer objetos del dominio.
La solución a este problema es sencilla:
- La factura representa para mi un comprobante de pago y nada más. Por lo tanto una vez generada no puede modificarse ya que tiene implicaciones contables (y quizás legales).
- Cuando el cliente llega al local y elige los productos que quiere no esta trabajando sobre una "factura", si no sobre una especie de "carrito de compras".
- Saber si se entregaron o no los productos no es responsabilidad de la factura, si no del sistema que lleva en control de las entregas.
Con este diseño la separación de responsabilidades es clara, no hay forma que en el código modifique items en una factura, o de pedirle el número de factura a un CarritoDeCompras, por lo tanto no hay necesidad de verificar un "estado".
El caso de "ENTREGADA" lo voy a discutir a continuación por que me sirve para ejemplificar otro "patrón" común.
Cambios de estado sin cambios de comportamiento
En el ejemplo el cambio de CarritoDeCompras a Factura implica un cambio en la semantica de los objetos. ¿Pero que pasa con el sello de "ENTREGADA"? ¿No se puede pensar como un simple flag booleano en una Factura?No quiero entrar en detalles de este dominio ficticio, pero quizás un "flag" no alcance. Es probable que quiera registrar quien realizo la entrega, a que hora, etc.
Por eso voy a hacer una simplificación: no me interesan esos detalles, solo quiero el equivalente al sello de "ENTREGADA".
Y dado que en este ejemplo yo pongo las reglas, voy a suponer que el operador en el mostrador de entregas tiene una pantalla que muestra las facturas pendientes de entrega, donde con un click puede cambiar el estado a "ENTREGADA".
Una forma de diseñar esto es pensar que uno tiene un "sistema" (modulo, o como deseen nombrar) que lleva el control de los pedidos, con dos "recipientes": uno para la facturas pedientes y otro para las entregadas. Entonces pasar de estado es mover la Factura de un recipiente a otro:
¿Pero el booleano no era mejor? No, este esquema tiene muchas ventajas más:
- Factura sigue siendo inmutable, e independiente de como llevo el control de entregas.
- SistemaDePedidos puede variar fácilmente, llevando por ejemplo el control de la fecha de entrega, etc.
¿Y la UI? "Quisiera hacer una página web que muestre el listado de facturas entregadas y no entregadas, con getEstado() o el flag es mucho más fácil".
En este caso también es fácil por que puede resolverse de la siguiente manera:
- La UI puede pedir al SistemaDePedidos directamente las Facturas entregadas o las no entregadas.
- Supongamos que la solución 1. no es satisfactoria: por como se utiliza la UI realmente se quiere tener un objeto al cual se le pueda pedir el estado. En ese caso conviene hacer un objeto especifico para la UI, este objeto puede verse como un DTO que se usa solo para transferir datos a la capa de UI.
¿Y la base de datos? "Tengo una tabla Factura que tiene la columna ENTREGADA"
No hay problema: las herramientas de mapeo O/R permiten hacer mapeos complejos de asociaciones para resolver el caso entregada/pendiente en el SistemaDePedidos, o si es necesario discriminar la subclase a mapear.
Conclusiones
En el ejemplo ficticio que di en este post aplica a muchos dominios, algunos ejemplos que me vienen a la mente son: estados de un documento (publicado/no publicado) en un sistema de CMS, registración de compras, distinción entre un estado de "edición/en uso" para una configuración (ejercicio: examinen el código fuente de Struts y vean como la implementación de configuración puede refactorizarse y mejorarse usando lo que comente en este articulo), etc.Creo que en la balanza de "bueno/malo" el código del estilo "if estado then codigo" tiene muchos problemas:
- Probablemente no modela adecuadamente el negocio.
- El "contrato" de los objetos es complicado: todo el tiempo dependo del estado para saber si puedo o no usar ciertos métodos.
- En consecuencia el código se empieza a "contaminar" con este tipo de chequeos, con el agravante que quizás cada programador lo haga de una forma distinta según la situación, dificultando el mantenimiento y refactoring.
- Se necesitan objetos mutables, en casos donde no conviene que lo sean (prometo hablar sobre "side effects" en otro post).
Mientras que las únicas ventajas que le veo son que es simple de implementar para ejemplos chicos y fácil de mapear a una tabla en la base de datos.
Por todas estas razones conviene evitar este tipo de "ifs" y en lo posible evitar el uso de Enums ya que fomentan este tipo de código.
Buenísimo el post Diego, muy didáctico y claro el ejemplo para solucionar el problema.
ResponderBorrarGracias por compartir!
en el capitulo 9 "Simplifying Conditional Expressions" del libro Refactoring se expone el patron "Replace Conditional with Polymorphism" para la problematica de los if's.
ResponderBorrarante las respuestas obtusas intento con recomendar la lectura de material muy preciso como el que mencione.
Diego, excelente post.
ResponderBorrar