Recientemente vi "All the Little Things" de RailsConf 2014. Durante esta charla, Sandi Metz refactoriza una función que incluye una gran instrucción if anidada:
def tick
if @name != 'Aged Brie' && @name != 'Backstage passes to a TAFKAL80ETC concert'
if @quality > 0
if @name != 'Sulfuras, Hand of Ragnaros'
@quality -= 1
end
end
else
...
end
...
end
El primer paso es dividir la función en varias más pequeñas:
def tick
case name
when 'Aged Brie'
return brie_tick
...
end
end
def brie_tick
@days_remaining -= 1
return if quality >= 50
@quality += 1
@quality += 1 if @days_remaining <= 0
end
Lo que encontré interesante fue la forma en que se escribieron estas funciones más pequeñas. brie_tick, por ejemplo, no se escribió extrayendo las partes relevantes de la tickfunción original , sino desde cero al referirse a las test_brie_*pruebas unitarias. Una vez que todas estas pruebas unitarias pasaron, brie_tickse consideró hecho. Una vez que se hicieron todas las funciones pequeñas, tickse eliminó la función monolítica original .
Desafortunadamente, el presentador parecía ignorar que este enfoque llevó a que tres de las cuatro *_tickfunciones estuvieran mal (¡y la otra estaba vacía!). Hay casos extremos en los que el comportamiento de las *_tickfunciones difiere del de la tickfunción original . Por ejemplo, @days_remaining <= 0in brie_tickdebe ser < 0, por lo brie_tickque no funciona correctamente cuando se llama con days_remaining == 1y quality < 50.
¿Qué ha salido mal aquí? ¿Es esto un fracaso de la prueba, porque no hubo pruebas para estos casos límite particulares? ¿O una falla de refactorización, porque el código debería haberse transformado paso a paso en lugar de reescribirse desde cero?
fuente
Respuestas:
Ambos. Refactorizar utilizando solo los pasos estándar del libro original de Fowlers es definitivamente menos propenso a errores que hacer una reescritura, por lo que a menudo es preferible usar solo este tipo de pequeños pasos. Incluso si no hay pruebas unitarias para cada caso límite, e incluso si el entorno no proporciona refactorizaciones automáticas, un solo cambio de código como "introducir variable explicativa" o "función de extracción" tiene una posibilidad mucho menor de cambiar los detalles de comportamiento del código existente que una reescritura completa de una función.
A veces, sin embargo, reescribir una sección de código es lo que necesita o desea hacer. Y si ese es el caso, necesita mejores pruebas.
Tenga en cuenta que incluso cuando se utiliza una herramienta de refactorización, siempre existe un cierto riesgo de introducir errores al cambiar el código, independientemente de aplicar pasos más pequeños o más grandes. Es por eso que la refactorización siempre necesita pruebas. Tenga en cuenta también que las pruebas solo pueden reducir la probabilidad de errores, pero nunca probar su ausencia; sin embargo, el uso de técnicas como mirar el código y la cobertura de la rama puede brindarle un alto nivel de confianza, y en caso de reescribir una sección de código, es A menudo vale la pena aplicar tales técnicas.
fuente
brie_ticksin tener que probar el@days_remaining == 1caso problemático , por ejemplo, probando con@days_remainingset to10y-10.Una de las cosas que es realmente difícil de trabajar con código heredado: adquirir una comprensión completa del comportamiento actual.
El código heredado sin pruebas que restringen todos los comportamientos es un patrón común en la naturaleza. Lo que te deja con una conjetura: ¿eso significa que los comportamientos sin restricciones son variables libres? o requisitos que no están especificados?
De la charla :
Este es el enfoque más conservador; Si los requisitos pueden estar subespecificados, si las pruebas no capturan toda la lógica existente, debe ser muy cuidadoso acerca de cómo proceder.
Por cierto, puede afirmar que si las pruebas describen de manera inadecuada el comportamiento del sistema, tiene una "falla de prueba". Y creo que es justo, pero en realidad no es útil; Este es un problema común en la naturaleza.
El problema no es que las transformaciones deberían haber sido paso a paso; sino que la elección de la herramienta de refactorización (¿operador de teclado humano? en lugar de automatización guiada) no estaba bien alineada con la cobertura de la prueba, debido a la mayor tasa de error.
Esto podría haber sido abordado ya sea mediante el uso de herramientas de refactorización con una mayor fiabilidad o mediante la introducción de una batería más amplia de pruebas para mejorar las restricciones del sistema.
Así que creo que tu conjunción está mal elegida;
ANDnoOR.fuente
La refactorización no debería cambiar el comportamiento visible externamente de su código. Ese es el objetivo.
Si sus pruebas unitarias fallan, eso indica que cambió el comportamiento. Pero pasar las pruebas unitarias nunca es el objetivo. Ayuda más o menos a lograr tu objetivo. Si la refactorización cambia el comportamiento visible externamente y todas las pruebas unitarias pasan, entonces su refactorización falló.
Las pruebas de unidades de trabajo en este caso solo le dan la sensación equivocada de éxito. ¿Pero qué salió mal? Dos cosas: la refactorización fue descuidada y las pruebas unitarias no fueron muy buenas.
fuente
Si define "correcto" como "la prueba pasa", entonces, por definición, no está mal cambiar el comportamiento no probado.
Si se debe definir un comportamiento de borde particular , agregue una prueba para ello, si no, entonces está bien que no le importe lo que suceda. Si es realmente pedante, puede escribir una prueba que verifique
truecuándo está en ese caso límite para documentar que no le importa cuál es el comportamiento.fuente