Código heredado de refactorización - Parte 11 ¿El fin?

En nuestra lección anterior, hemos aprendido una nueva forma de entender y mejorar el código extrayendo hasta que lo abandonamos. Si bien ese tutorial era una buena manera de aprender las técnicas, difícilmente era el ejemplo ideal para entender los beneficios de la misma. En esta lección extraeremos hasta que coloquemos todos nuestros códigos relacionados con juegos de trivia y analizaremos el resultado final.

Esta lección también concluirá nuestra serie sobre refactorización. Si crees que nos perdimos algo, no dudes en comentar con un tema propuesto. Si se reúnen buenas ideas, continuaré con tutoriales adicionales basados ​​en sus solicitudes.


Atacar a nuestro método más largo

Qué mejor manera de comenzar nuestro artículo que tomando nuestro método más largo y extrayéndolo en pequeños pedazos. Las pruebas primero, como de costumbre, harán que este procedimiento no solo sea eficiente sino también divertido.

Como de costumbre, tiene el código tal como estaba cuando comencé este tutorial en el php_start carpeta, mientras que el resultado final está en la php carpeta.

funcion wasCorrectlyAnswered () if ($ this-> inPenaltyBox [$ this-> currentPlayer]) if ($ this-> isGettingOutOfPenaltyBox) $ this-> display-> correctAnswer (); $ this-> purses [$ this-> currentPlayer] ++; $ this-> display-> playerCoins ($ this-> players [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ winner = $ this-> didPlayerNotWin (); $ this-> currentPlayer ++; if ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0;  devuelve $ ganador;  else $ this-> currentPlayer ++; if ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0;  devuelve true;  else else $ this-> display-> correctAnswerWithTypo (); $ this-> purses [$ this-> currentPlayer] ++; $ this-> display-> playerCoins ($ this-> players [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ winner = $ this-> didPlayerNotWin (); $ this-> currentPlayer ++; if ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0;  devuelve $ ganador; 

Este método, fue respondido correctamente (), es nuestra primera victima.


Obtener una respuesta correcta fue respondida ()

Como aprendimos de las lecciones anteriores, el primer paso para modificar el código heredado es ponerlo a prueba. Esto puede ser un proceso difícil. Afortunadamente para nosotros, el fue respondido correctamente () El método es bastante sencillo. Se compone de varios si-si no declaraciones Cada rama del código devuelve un valor. Cuando tenemos un valor de retorno, siempre podemos especular que la prueba es factible. No es necesario fácil, pero al menos posible..

function testWasCorrectlyAnsweredAndGettingOutOfPenaltyBoxWhileBeingAWinner () $ this-> setAPlayerThatIsInThePenaltyBox (); $ this-> game-> isGettingOutOfPenaltyBox = true; $ this-> game-> purses [$ this-> game-> currentPlayer] = Game :: $ numberOfCoinsToWin; $ this-> assertTrue ($ this-> game-> wasCorrectlyAnswered ()); 

No hay una regla definida sobre qué prueba escribir primero. Acabamos de elegir el primer camino de ejecución aquí. De hecho, tuvimos una agradable sorpresa y reutilizamos uno de los métodos privados que extrajimos muchos tutoriales antes. Pero aún no hemos terminado. Todo verde, así que es hora de refactorizar..

function testWasCorrectlyAnsweredAndGettingOutOfPenaltyBoxWhileBeingAWinner () $ this-> setAPlayerThatIsInThePenaltyBox (); $ this-> currentPlayerWillLeavePenaltyBox (); $ this-> setCurrentPlayerAWinner (); $ this-> assertTrue ($ this-> game-> wasCorrectlyAnswered ()); 

Esto es más fácil de leer y significativamente más descriptivo. Puedes encontrar los métodos extraídos en el código adjunto..

function testWasCorrectlyAnsweredAndGettingOutOfPenaltyBoxWhile NOTBeingAWinner () $ this-> setAPlayerThatIsInThePenaltyBox (); $ this-> currentPlayerWillLeavePenaltyBox (); $ this-> setCurrentPlayerNotAWinner (); $ this-> assertFalse ($ this-> game-> wasCorrectlyAnswered ());  la función privada setCurrentPlayerNotAWinner () $ this-> game-> purses [$ this-> game-> currentPlayer] = 0; 

Esperábamos que esto pasara, pero falla. Las razones no están del todo claras. Una mirada más cercana a didPlayerNotWin () puede ser útil.

function didPlayerNotWin () return! ($ this-> purses [$ this-> currentPlayer] == self :: $ numberOfCoinsToWin); 

El método devuelve verdadero cuando un jugador no ganó. Tal vez podríamos cambiar el nombre de nuestra variable pero primero, las pruebas deben pasar.

función privada setCurrentPlayerAWinner () $ this-> game-> purses [$ this-> game-> currentPlayer] = Game :: $ numberOfCoinsToWin;  la función privada setCurrentPlayerNotAWinner () $ this-> game-> purses [$ this-> game-> currentPlayer] = 0; 

En una inspección más cercana, podemos ver que mezclamos los valores aquí. Nuestra confusión entre el nombre del método y el nombre de la variable nos hizo revertir las condiciones.

función privada setCurrentPlayerAWinner () $ this-> game-> purses [$ this-> game-> currentPlayer] = 0;  la función privada setCurrentPlayerNotAWinner () $ this-> game-> purses [$ this-> game-> currentPlayer] = Game :: $ numberOfCoinsToWin - 1; 

Esto esta funcionando Mientras se analiza didPlayerNotWin () También observamos que utiliza la igualdad para determinar el ganador. Debemos establecer nuestro valor en uno menos porque el valor se incrementa en el código de producción que probamos.

Las tres pruebas restantes son fáciles de escribir. Son solo variaciones de los dos primeros. Puedes encontrarlos en el código adjunto..


Extraer y renombrar hasta que eliminemos el método wasCorrectlyAnswered ()

El problema más confuso es el engañoso. $ ganador nombre de la variable. Eso debería ser $ notAWinner.

$ notAWinner = $ this-> didPlayerNotWin (); $ this-> currentPlayer ++; if ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0;  devolver $ notAWinner; 

Podemos observar que la $ notAWinner La variable se usa solo para devolver un valor. Podriamos llamar al didPlayerNotWin () método directamente en la declaración de retorno?

$ this-> currentPlayer ++; if ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0;  devolver $ this-> didPlayerNotWin (); 

Esto aún pasa nuestra prueba de unidad, pero si ejecutamos nuestras pruebas Golden Master, fallarán con el error "no hay suficiente memoria". De hecho, el cambio hace que el juego nunca termine..

Lo que sucede es que el jugador actual se actualiza al siguiente jugador. Como teníamos un solo jugador, siempre reutilizábamos al mismo jugador. Así es la prueba. Nunca se sabe cuando descubre una lógica oculta en un código difícil..

function testWasCorrectlyAnsweredAndGettingOutOfPenaltyBoxWhileBeingAWinner () $ this-> setAPlayerThatIsInThePenaltyBox (); $ this-> game-> add ('Another Player'); $ this-> currentPlayerWillLeavePenaltyBox (); $ this-> setCurrentPlayerAWinner (); $ this-> assertTrue ($ this-> game-> wasCorrectlyAnswered ()); 

Con solo agregar otro jugador en cada una de nuestras pruebas relacionadas con este método, podemos asegurarnos de que la lógica está cubierta. Esta prueba hará que la declaración de devolución modificada anteriormente falle..

función privada selectNextPlayer () $ this-> currentPlayer ++; if ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; 

Podemos detectar inmediatamente que la selección del siguiente jugador es idéntica en ambos caminos de la condición. Podemos moverlo a un método propio. El nombre que elegimos para este método es seleccioneNextPlayer (). Este nombre ayuda a resaltar el hecho de que se cambiará el valor del jugador actual. También sugiere que didPlayerNotWin () podría ser renombrado en algo que refleje la relación con el jugador actual.

funcion wasCorrectlyAnswered () if ($ this-> inPenaltyBox [$ this-> currentPlayer]) if ($ this-> isGettingOutOfPenaltyBox) $ this-> display-> correctAnswer (); $ this-> purses [$ this-> currentPlayer] ++; $ this-> display-> playerCoins ($ this-> players [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ notAWinner = $ this-> didCurrentPlayerNotWin (); $ this-> selectNextPlayer (); devuelve $ notAWinner;  else $ this-> selectNextPlayer (); devuelve verdadero  else else $ this-> display-> correctAnswerWithTypo (); $ this-> purses [$ this-> currentPlayer] ++; $ this-> display-> playerCoins ($ this-> players [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ notAWinner = $ this-> didCurrentPlayerNotWin (); $ this-> selectNextPlayer (); devuelve $ notAWinner; 

Nuestro código es cada vez más corto y más expresivo. ¿Qué podríamos hacer a continuación? Podríamos cambiar el nombre extraño de la lógica "no ganador" y cambiar el método a una lógica positiva en lugar de una negativa. O podríamos continuar extrayendo y lidiar con la confusión lógica negativa más tarde. No creo que haya una manera definitiva de ir. Por lo tanto, dejaré el problema de la lógica negativa como un ejercicio para usted y continuaremos con la extracción..

funcion wasCorrectlyAnswered () if ($ this-> inPenaltyBox [$ this-> currentPlayer]) return $ this-> getCorrectlyAnsweredForPlayersInPenaltyBox ();  else return $ this-> getCorrectlyAnsweredForPlayersNotInPenaltyBox (); 
Como regla general, intente tener una sola línea de código en cada ruta de una lógica de decisión.

Extrajimos todo el bloque de código en cada parte de nuestro Si declaración. Este es un paso importante, y algo en lo que siempre debe pensar. Cuando tiene una ruta de decisión o un bucle en su código, dentro de él debe haber una sola declaración. La persona que lea este método probablemente no se preocupará por los detalles de la implementación. Él o ella se preocupará por la lógica de decisión, la Si declaración.

funcion wasCorrectlyAnswered () if ($ this-> inPenaltyBox [$ this-> currentPlayer]) return $ this-> getCorrectlyAnsweredForPlayersInPenaltyBox ();  devolver $ this-> getCorrectlyAnsweredForPlayersNotInPenaltyBox (); 

Y si podemos deshacernos de cualquier código extra, deberíamos. La eliminación de la más Y aún manteniendo la lógica igual hicimos una pequeña economía. Me gusta más esta solución porque resalta cuál es el comportamiento "predeterminado" de la función. El código que se encuentra directamente en el interior de la función (la última línea de código aquí). los Si sentencia es la funcionalidad excepcional añadida a la predeterminada..

Escuché razonamientos de que escribir las condiciones de esta manera puede ocultar el hecho de que el valor predeterminado no se ejecuta si el Si Se activa la declaración. Solo puedo estar de acuerdo con esto, y si prefieres mantener el más parte por claridad, por favor hazlo.

función privada getCorrectlyAnsweredForPlayersInPenaltyBox () if ($ this-> isGettingOutOfPenaltyBox) return $ this-> getCorrectlyAnsweredForPlayerGettingOutOfPenaltyBox ();  else return $ this-> getCorrectlyAnsweredForPlayerStayingInPenaltyBox (); 

Podemos seguir extrayendo dentro de nuestros nuevos métodos privados creados. Aplicar el mismo principio a nuestra siguiente declaración condicional conduce al código anterior.

función privada giveCurrentUserACoin () $ this-> purses [$ this-> currentPlayer] ++; 

Mirando nuestros métodos privados. getCorrectlyAnsweredForPlayersNotInPenaltyBox () y getCorrectlyAnsweredForPlayerGettingOutOfPenaltyBox () Podemos observar inmediatamente que una tarea simple está duplicada. Esa asignación puede ser obvia para alguien como nosotros que ya sabe lo que hay con las carteras y las monedas, pero no para un recién llegado. Extracción de esa sola línea en un método giveCurrentUserACoin () es una buena idea.

Ayuda con la duplicación también. Si en el futuro cambiamos la forma en que otorgamos monedas a los jugadores, necesitaremos cambiar el código solo dentro de este método privado..

función privada getCorrectlyAnsweredForPlayersNotInPenaltyBox () $ this-> display-> correctAnswerWithTypo (); devuelve $ this-> getCorrectlyAnsweredForAPlayer ();  función privada getCorrectlyAnsweredForPlayerGettingOutOfPenaltyBox () $ this-> display-> correctAnswer (); devuelve $ this-> getCorrectlyAnsweredForAPlayer ();  función privada getCorrectlyAnsweredForAPlayer () $ this-> giveCurrentUserACoin (); $ this-> display-> playerCoins ($ this-> players [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ notAWinner = $ this-> didCurrentPlayerNotWin (); $ this-> selectNextPlayer (); devuelve $ notAWinner; 

Entonces, los dos métodos respondidos correctamente son idénticos, excepto que uno de ellos produce algo con un error tipográfico. Extrajimos el código duplicado y conservamos las diferencias en cada método. Puede pensar que podríamos haber usado el método extraído con un parámetro en el código del llamante y haber generado una vez normalmente y una vez con un error tipográfico. Sin embargo, la solución propuesta anteriormente tiene una ventaja: mantiene separados los dos conceptos de no en cuadro de penalización y salir del cuadro de penalización.

Con esto concluye el trabajo sobre fue respondido correctamente ().


¿Qué pasa con el método worngAnswer ()?

function wrongAnswer () $ this-> display-> incorrectAnswer (); $ currentPlayer = $ this-> players [$ this-> currentPlayer]; $ this-> display-> playerSentToPenaltyBox ($ currentPlayer); $ this-> inPenaltyBox [$ this-> currentPlayer] = true; $ this-> currentPlayer ++; if ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0;  devuelve true; 

En 11 líneas este método no es enorme, pero sí grande. ¿Recuerdas el número mágico siete más menos dos investigaciones? Afirma que nuestro cerebro puede pensar simultáneamente en 7 + -2 cosas. Es decir, tenemos una capacidad limitada. Entonces, para entender un método de manera fácil y completa, queremos que la lógica dentro de él se ajuste a ese rango. Con un total de 11 líneas y un contenido de 9 líneas, este método se encuentra al límite. Puedes argumentar que en realidad hay una línea vacía y otra con solo un refuerzo. Eso lo haría solo 7 líneas de lógica..

Si bien las llaves y los espacios son cortos en el espacio, tienen un significado para nosotros. Separan partes de la lógica, tienen un significado, por lo que nuestro cerebro debe procesarlos. Sí, es más fácil en comparación con una línea completa o lógica de comparación, pero aún así.

Es por eso que nuestro objetivo para las líneas de lógica dentro de un método es 4 líneas. Eso está por debajo del mínimo de la teoría anterior, por lo que tanto un genio como un programador mediocre deberían poder comprender el método.

$ this-> currentPlayer ++; if ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; 

Ya tenemos un método para este código, por lo que deberíamos usarlo..

function wrongAnswer () $ this-> display-> incorrectAnswer (); $ currentPlayer = $ this-> players [$ this-> currentPlayer]; $ this-> display-> playerSentToPenaltyBox ($ currentPlayer); $ this-> inPenaltyBox [$ this-> currentPlayer] = true; $ this-> selectNextPlayer (); devuelve verdadero 

Mejor, pero debemos soltar o continuar?

$ currentPlayer = $ this-> players [$ this-> currentPlayer]; $ this-> display-> playerSentToPenaltyBox ($ currentPlayer);

Podríamos alinear la variable desde estas dos líneas.. $ this-> currentPlayer Obviamente está devolviendo al jugador actual, así que no hay necesidad de repetir la lógica. No aprendemos nada nuevo ni abstraemos nada nuevo usando la variable local.

function wrongAnswer () $ this-> display-> incorrectAnswer (); $ this-> display-> playerSentToPenaltyBox ($ this-> players [$ this-> currentPlayer]); $ this-> inPenaltyBox [$ this-> currentPlayer] = true; $ this-> selectNextPlayer (); devuelve verdadero 

Estamos a 5 líneas. Cualquier otra cosa allí?

$ this-> inPenaltyBox [$ this-> currentPlayer] = true;

Podemos extraer la línea anterior en su propio método. Esto ayudará a explicar lo que está sucediendo y aislará la lógica de enviar al jugador actual al cuadro de penalización en su propio lugar.

function wrongAnswer () $ this-> display-> incorrectAnswer (); $ this-> display-> playerSentToPenaltyBox ($ this-> players [$ this-> currentPlayer]); $ this-> sendCurrentPlayerToPenaltyBox (); $ this-> selectNextPlayer (); devuelve verdadero 

Todavía 5 líneas, pero todas las llamadas de método. Los dos primeros están mostrando cosas. Los dos siguientes están relacionados con nuestra lógica. La última línea devuelve true. No veo ninguna manera de hacer que este método sea más fácil de entender sin introducir complejidad mediante las extracciones que podríamos realizar, por ejemplo, extrayendo los dos métodos de visualización en uno privado. Si quisiéramos hacerlo, ¿dónde debería ir ese método? Dentro de esto Juego clase, o en Monitor? Creo que esta ya es una pregunta demasiado compleja para merecer ser considerada con respecto a la simplicidad de nuestro método..


Pensamientos finales y algunas estadísticas

Hagamos algunas estadísticas revisando esta gran herramienta del escritor de PHPUnit https://github.com/sebastianbergmann/phploc.git

Estadísticas en el código de juego trivia original

./ phploc… / Refactoring \ Legacy \ Code \ - \ Part \ 1 \: \ The \ Golden \ Master / Source / trivia / php / phploc 2.1-gca70e70 por Sebastian Bergmann. Tamaño de líneas de código (LOC) 232 Líneas de código de comentarios (CLOC) 0 (0.00%) Líneas de código sin comentarios (NCLOC) 232 (100.00%) Líneas de código lógicas (LLOC) 99 (42.67%) Clases 88 (88.89 %) Longitud de clase promedio 88 Longitud de clase mínima 88 Longitud de clase máxima 88 Longitud de método promedio 7 Longitud de método mínima 1 Longitud de método máxima 17 Funciones 1 (1.01%) Longitud de función promedio 1 No en clases o funciones 10 (10.10%) Complejidad ciclomática Complejidad promedio por LLOC 0.26 Complejidad promedio por clase 25.00 Complejidad de clase mínima 25.00 Complejidad de clase máxima 25.00 Complejidad promedio por método 3.18 Complejidad de método mínima 1.00 Complejidad de método máxima 10.00 Dependencias Accesos globales 0 Constantes globales 0 (0.00%) Variables globales 0 (0.00%) Super-Global Variables 0 (0.00%) Atributos Accesos 115 No estático 115 (100.00%) Estático 0 (0.00%) Llamadas a métodos 21 No estático 21 (100.00%) Estático 0 (0.00%) Espacios de nombres de estructuras 0 Interfaces 0 Rasgos 0 Clases 1 Resumen Clases 0 (0.00%) Clases concretas 1 ( 100.00%) Métodos 11 Alcance Métodos no estáticos 11 (100.00%) Métodos estáticos 0 (0.00%) Visibilidad Métodos públicos 11 (100.00%) Métodos no públicos 0 (0.00%) Funciones 1 Funciones nombradas 1 (100.00%) Funciones anónimas 0 (0.00%) Constantes 0 Global Constantes 0 (0.00%) Constantes de clase 0 (0.00%)

Estadísticas en el código de juego de Trivia Refactored

./ phploc… / Refactoring \ Legacy \ Code \ - \ Part \ 11 \: \ The \ End \? / Source / trivia / php phploc 2.1-gca70e70 por Sebastian Bergmann. Tamaño de líneas de código (LOC) 371 Líneas de código de comentarios (CLOC) 0 (0.00%) Líneas de código sin comentarios (NCLOC) 371 (100.00%) Líneas de código lógicas (LLOC) 151 (40.70%) Clases 145 (96.03 %) Longitud media de la clase 36 Longitud mínima de la clase 8 Longitud máxima de la clase 89 Longitud media del método 2 Longitud mínima del método 1 Longitud máxima del método 14 Funciones 0 (0.00%) Longitud media de la función 0 No en clases o funciones 6 (3.97%) Complejidad ciclomática Complejidad promedio por LLOC 0.15 Complejidad promedio por clase 6.50 Complejidad de clase mínima 1.00 Complejidad de clase máxima 17.00 Complejidad promedio por método 1.46 Complejidad de método mínima 1.00 Complejidad de método máxima 10.00 Dependencias Accesos globales 0 Constantes globales 0 (0.00%) Variables globales 0 (0.00%) Super-Global Variables 0 (0.00%) Atributos Accesos 96 No estático 94 (97.92%) Estático 2 (2.08%) Llamadas al método 74 No estático 74 (100.00%) Estático 0 (0.00%) Estructura Espacios de nombres 0 Interfaces 1 Rasgos 0 Clases 3 Resumen Clases 0 (0.00%) Clases de concreto 3 (100.00) %) Métodos 59 Alcance Métodos no estáticos 59 (100.00%) Métodos estáticos 0 (0.00%) Visibilidad Métodos públicos 35 (59.32%) Métodos no públicos 24 (40.68%) Funciones 0 Funciones con nombre 0 (0.00%) Funciones anónimas 0 (0.00%) Constantes 3 Constantes globales 0 (0.00%) Constantes de clase 3 (100.00%) 

Analiza

Los datos brutos son tan buenos como podemos entenderlos y analizarlos.

La cantidad de líneas lógicas de código aumentó significativamente de 99 a 151. Pero esta cantidad no debería engañarlo para que piense que nuestro código se volvió más complejo. Esta es una tendencia natural de código bien refactorizado, debido al aumento en el número de métodos y llamadas a ellos..

Tan pronto como observamos la longitud promedio de la clase, podemos ver una caída dramática en las líneas de código, de 88 a 36.

Y es sorprendente cómo la longitud del método bajó de un promedio de siete líneas a solo dos líneas de código..

Si bien el número de líneas es un buen indicador del volumen de código por unidad de medida, la ganancia real se encuentra en los análisis de la complejidad ciclomática. Cada vez que tomamos una decisión en nuestro código, aumentamos la complejidad ciclomática. Cuando encadenamos Si Enunciados uno dentro del otro, la complejidad ciclomática de ese método aumenta exponencialmente. Nuestras extracciones continuas llevaron a métodos con una sola decisión, lo que redujo su complejidad promedio por método de 3.18 a 1.00. Puede leer esto como "nuestros métodos refactorizados son 3.18 veces más simples que el código original". A nivel de clase, la caída en la complejidad es aún más sorprendente. Descendió de 25.00 a 6.50..

El fin?

Bien. Eso es. Fin de la serie. Por favor, siéntase libre de expresar sus opiniones y, si cree que nos hemos perdido algún tema de refactorización, pídalo en los comentarios a continuación. Si son interesantes los transformaré en partes extra de esta serie..

Gracias por su atención indivisa.