Hace unos días dediqué un rato a programar junto con Xavi en una sesión de refactor para mejorar una pieza de código similar a la que expondré a lo largo de post. La idea principal era sustituir una estructura if-else por algo más legible, mantenible, elegante y más alineado con los principios SOLID. El entramado de if-else al que nos enfrentábamos era más grande que el que expongo, lo cual hacía la necesidad de refactor aún más latente.

Digamos que tenemos una estructura parecida a esta:

En función de ciertas condiciones sobre “item1″ e “item2″ se devolvía un procesador que realizaría determinadas operaciones sobre ellos. Además algún procesador necesitaba de “collaborator” para sus operaciones.

Pues bien, en primer lugar lo que hicimos fue “aplanar” la estructura if-else para que tuviese solo un nivel y que fuese más fácil de seguir el flujo de condiciones y resultados.

Además vimos que no era necesaria la parte de los else ya que si cada condición se cumplía se hacía return. Así que también lo eliminamos.

En este momento la idea fué eliminar la cadena de ifs construyendo una serie de objectos que encapsulasen la lógica de conocer si se cumplen ciertas condiciones sobre los parámetros de entrada y saber el procesador concreto a devolver. Todos implementarán una interfaz de manera que podamos agruparlos en una estructura que se pueda recorrer buscando el procesador concreto en función de los parámetros. Empezamos con la primera rama de if. En ella vemos que no se necesita item2, pero lo pasamos de todos modos para cumplir con la interfaz Creator.

La idea es que la factoría de procesadores contenga un conjunto de objectos que conozcan la condición necesario para devolver un procesador concreto. De esa manera el método getConcreteProcessor recorrerá dicha lista hasta encontrar el primer objecto que le devuelva un procesador.
Continuamos con el resto de condiciones y procesadores.

En este punto hay que resaltar un par de asuntos. Uno de ellos es que necesitamos pasar al ConcreteProcessor3Creator el parámetro “collaborator” a pesar de que no se utiliza, pero lo hacemos para que pueda cumplir la interfaz. Es un mal menor y probablemente señal de que todavía se podría iterar más sobre este código para ver de dónde surge esta necesidad. Por otro lado, hemos introducido algo de duplicación al tener el método “condition1″ en dos clases anónimas diferentes. Así que dándole otra vuelta más vimos que podríamos crear un objeto que englobase Item1 e Item2, ya que parece que son dos objetos que van bastante unidos en este flujo. De esta manera además de reducir el número de parámetros podemos agrupar todos los métodos “condition” dentro de esa especie de “wrapper”.

De esta manera hemos englobado toda la lógica que tiene que ver con condiciones sobre los objetos “item1″ e “item2″ en una sóla clase y hemos eliminado la duplicación del método “condition1″. Además, por último, el método “getConcreteProcessor” sería llamado directamente con el objeto “extendedItem” ya construido. Un parámetro menos.

Ahora toca reflexionar. Recuerdo que le preguntaba a Xavi “y de verdad merece la pena todo ‘este pitote’ para deshacernos de un if?”. La respuesta, por supuesto, era que sí. Sobre todo porque nuestro if era más grande y con posibilidades de crecer aún más.
- Hemos distribuido mejor las responsabilidades, el código está mucho más limpio y es más fácil de entender.
- Se pueden añadir otros procesadores en base a otras condiciones de manera más sencilla, sin necesidad de extender aún más la estructura if-else.
- El método “getConcreteProcessor” es mucho más pequeño y ya no necesita conocer nada sobre las clases Item1 e Item2 ni sobre condiciones que aplican sobre ellas.

Otra de mis preguntas era si debíamos sacar cada una de esas inner clases a sus propios ficheros, siendo la respuesta que no, ya que están directamente ligadas a la propia factoría.

Casi se me olvida! Esto es muy “de Xavi”! Invertir las condiciones en los creator para que quede más en modo cláusula de guarda cuando no haya que devolver procesador.