Por Luis Rodriguez
En LeanMind, siempre estamos buscando la manera de mejorar nuestras habilidades. Una forma que nos encanta, es por medio de la práctica deliberada, es decir, con ejercicios o también llamados katas, haciéndolos de manera consciente, buscando un objetivo concreto.
En este caso, queremos mejorar nuestras habilidades de refactoring y que mejor que con la kata Gilded Rose de la mano de Emily Bache.
Partimos del siguiente código escrito en kotlin
:
class GildedRose(var items: List<Item>) {
fun updateQuality() {
for (i in items.indices) {
if (items[i].name != "Aged Brie" && items[i].name != "Backstage passes to a TAFKAL80ETC concert") {
if (items[i].quality > 0) {
if (items[i].name != "Sulfuras, Hand of Ragnaros") {
items[i].quality = items[i].quality - 1
}
}
} else {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1
if (items[i].name == "Backstage passes to a TAFKAL80ETC concert") {
if (items[i].sellIn < 11) {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1
}
}
if (items[i].sellIn < 6) {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1
}
}
}
}
}
if (items[i].name != "Sulfuras, Hand of Ragnaros") {
items[i].sellIn = items[i].sellIn - 1
}
if (items[i].sellIn < 0) {
if (items[i].name != "Aged Brie") {
if (items[i].name != "Backstage passes to a TAFKAL80ETC concert") {
if (items[i].quality > 0) {
if (items[i].name != "Sulfuras, Hand of Ragnaros") {
items[i].quality = items[i].quality - 1
}
}
} else {
items[i].quality = items[i].quality - items[i].quality
}
} else {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1
}
}
}
}
}
Lo primero en lo que nos podemos fijar es en la cantidad de condicionales que existen, haciendo que en código haya muchos valles y montañas.
A primera vista es un código complicado de entender. Antes de hacer cualquier cambio, lo que hay que hacer es, tejer una buena red de seguridad con pruebas unitarias, para que cuando hagamos algún cambio en el código de producción, no estemos introduciendo algún bug.
De todo el catálogo que ofrece Martin Fowler en su libro Refactoring, creo que lo más apropiado es ir reduciendo la indentación, porque de esa manera vamos disminuyendo la carga cognitiva, haciendo que el código sea más legible para el lector. Otras recetas también creo que se pueden aplicar, como el extract method
o extract variable
entre otras, pero creo que en este caso concreto, ir reduciendo la rama de los else`s en los condicionales, va a tener un mayor impacto en la legibilidad del código.
Hay que tener en cuenta que un método puede tener varios puntos de salida. A lo mejor puede chocar en un principio. Lo comento, porque no hace muchos años se enseñaba que los programas/métodos/funciones, tenían que tener un punto de entrada y un punto de salida. Esto tenía sentido, porque al programar en ensamblador/C/Pascal o similares
, los métodos/rutinas
solían ser bastante grandes, conteniendo muchas líneas de código y con muchas bifurcaciones en su interior, pero ahora con los lenguajes modernos de la actualidad, no es necesario seguir esta regla. Incluso es preferible lo pequeño y acotado, a lo grande y multifuncional. Un concepto muy interesante que podemos aplicar es el de clausula de guarda
.
Antes de nada he aplicado el refactor extract method
, porque es mejor centrarse en un item
, que en muchos a la vez, de esta forma, reducimos la carga cognitiva y las mejoras se pueden hacer de una manera más cómoda.
Aplicando varias veces este cambio que hemos comentado anteriormente (reducir la indentación, quitando los else’s), nos podemos encontrar con el siguiente código:
class GildedRose(var items: List<Item>) {
fun updateQuality() {
for (item in items) {
updateQuality(item)
}
}
private fun updateQuality(item: Item) {
if (item.name == "Sulfuras, Hand of Ragnaros") {
return
}
if (item.name == "Backstage passes to a TAFKAL80ETC concert") {
if (item.quality < 50) {
item.quality = item.quality + 1
}
if (item.sellIn < 11) {
if (item.quality < 50) {
item.quality = item.quality + 1
}
}
item.sellIn = item.sellIn - 1
if (item.sellIn < 0) {
item.quality = 0
}
return
}
if (item.name == "Aged Brie") {
if (item.quality < 50) {
item.quality = item.quality + 1
}
item.sellIn = item.sellIn - 1
if (item.sellIn < 0) {
if (item.quality < 50) {
item.quality = item.quality + 1
}
}
return
}
if (item.quality > 0) {
item.quality = item.quality - 1
}
item.sellIn = item.sellIn - 1
if (item.sellIn < 0) {
if (item.quality > 0) {
item.quality = item.quality - 1
}
}
}
Viendo el código, ya podemos percibir que hay varios tipos de elementos, que se discriminan por su propiedad nombre. Aplicando el refactor extract method
y un when/switch
, en vez varios if's
, podemos ver el siguiente resultado:
class GildedRose(var items: List<Item>) {
fun updateQuality() {
for (item in items) {
updateQuality(item)
}
}
private fun updateQuality(item: Item) {
when (item.name) {
"Sulfuras, Hand of Ragnaros" -> {
}
"Backstage passes to a TAFKAL80ETC concert" -> {
updateBackstage(item)
}
"Aged Brie" -> {
updateAgedBrie(item)
}
else -> updateNormal(item)
}
}
private fun updateNormal(item: Item) {
if (item.quality > 0) {
item.quality = item.quality - 1
}
item.sellIn = item.sellIn - 1
if (item.sellIn < 0) {
if (item.quality > 0) {
item.quality = item.quality - 1
}
}
}
private fun updateAgedBrie(item: Item) {
if (item.quality < 50) {
item.quality = item.quality + 1
}
item.sellIn = item.sellIn - 1
if (item.sellIn < 0) {
if (item.quality < 50) {
item.quality = item.quality + 1
}
}
}
private fun updateBackstage(item: Item) {
if (item.quality < 50) {
item.quality = item.quality + 1
}
if (item.sellIn < 11) {
if (item.quality < 50) {
item.quality = item.quality + 1
}
}
item.sellIn = item.sellIn - 1
if (item.sellIn < 0) {
item.quality = 0
}
}
}
Se puede seguir mejorando el código, aplicando el patrón estrategia, pero me quería centrar en la indentación del código como olor para aplicar las mejoras.
Hay que tener cuidado con las condiciones, porque es muy fácil aumentar la carga cognitiva, haciendo un código de muy poca calidad.
Happy Coding !!!
Referencias:
Código Sostenible, página 108
Implementation Patterns, página 97
¿Quieres más? te invitamos a suscribirte a nuestro boletín para avisarte cada vez que recopilemos contenido de calidad que compartir.
Si disfrutas leyendo nuestro blog, ¿imaginas lo divertido que sería trabajar con nosotros? ¿te gustaría?
Pero espera 🖐 que tenemos un conflicto interno. A nosotros las newsletter nos parecen 💩👎👹 Por eso hemos creado la LEAN LISTA, la primera lista zen, disfrutona y que suena a rock y reggaeton del sector de la programación. Todos hemos recibido newsletters por encima de nuestras posibilidades 😅 por eso este es el compromiso de la Lean Lista