Código heredado de refactorización Parte 2 - Cadenas y constantes mágicas

Código antiguo. Código feo Código complicado. Código de espagueti. Tonterías jibberish. En dos palabras, Código Legado. Esta es una serie que te ayudará a trabajar y lidiar con ella..

Primero conocimos nuestro código fuente heredado en nuestra lección anterior. Luego ejecutamos el código para formarse una opinión acerca de su funcionalidad básica y creamos una suite de prueba Golden Master para tener una red de seguridad en bruto para futuros cambios. Continuaremos trabajando en este código y lo puede encontrar en el trivia / php_start carpeta. La otra carpeta trivia / php_start es con el código terminado de esta lección.

¿Ha llegado el momento de los primeros cambios y qué mejor manera de entender una base de código difícil que comenzar a extraer constantes mágicas y cadenas en variables? Estas tareas aparentemente simples nos darán una visión mayor y, en ocasiones, inesperada, del funcionamiento interno del código heredado. Tendremos que descubrir las intenciones del autor del código original y encontrar los nombres adecuados para las piezas de código que nunca hemos visto antes.

Cuerdas mágicas

Las cadenas mágicas son cadenas usadas directamente en varias expresiones, sin ser asignadas a una variable. Este tipo de cadena tenía un significado especial para el autor original del código, pero en lugar de asignarlas a una variable bien nombrada, el autor pensó que el significado de la cadena era lo suficientemente obvio..

Identificar las primeras cadenas para cambiar

Comencemos mirando nuestro Game.php y tratar de identificar las cadenas. Si está utilizando un IDE (y debería) o un editor de texto más inteligente capaz de resaltar el código fuente, detectar las cadenas será fácil. Aquí hay una imagen de cómo se ve el código en mi pantalla.

Todo con naranja es una cuerda. Encontrar cada cadena en nuestro código fuente es muy fácil de esta manera. Por lo tanto, asegúrese de que el resaltado sea compatible y esté habilitado en su editor, independientemente de su aplicación..

La primera parte naranja en nuestro código está inmediatamente en la línea tres. Sin embargo, la cadena sólo contiene un carácter de nueva línea. Esto debería ser lo suficientemente obvio en mi opinión, para que podamos seguir adelante.

Cuando se trata de decidir qué extraer y qué mantener sin cambios, hay pocas revisiones, pero al final es su opinión profesional la que debe prevalecer. Sobre esta base, tendrá que decidir qué hacer con cada pieza de código que analice..

para ($ i = 0; $ i < 50; $i++)  array_push($this->PopQuestions, "Pop Question". $ i); array_push ($ this-> scienceQuestions, ("Science Question". $ i)); array_push ($ this-> sportsQuestions, ("Sports Question". $ i)); array_push ($ this-> rockQuestions, $ this-> createRockQuestion ($ i));  function createRockQuestion ($ index) return "Rock Question". $ índice; 

Así que analicemos las líneas 32 a 42, el fragmento que puede ver arriba. Para preguntas sobre el pop, la ciencia y los deportes, solo hay una concatenación simple. Sin embargo, la acción para componer la cadena para una pregunta rock se extrae en un método. En su opinión, ¿son estas concatenaciones y cadenas lo suficientemente claras para que podamos mantenerlas todas dentro de nuestro bucle for? O, ¿cree que la extracción de todas las cadenas en sus métodos justificaría la existencia de esos métodos? Si es así, ¿cómo nombrarías esos métodos??

Actualizar Golden Master y las pruebas

Independientemente de la respuesta, necesitaremos modificar el código. Es hora de poner a trabajar a nuestro Maestro Dorado y escribir nuestra prueba que realmente ejecuta y compara nuestro código con el contenido existente.

la clase GoldenMasterTest extiende PHPUnit_Framework_TestCase private $ gmPath; función setUp () $ this-> gmPath = __DIR__. '/gm.txt';  function testGenerateOutput () $ times = 20000; $ this-> generateMany ($ times, $ this-> gmPath);  function testOutputMatchesGoldenMaster () $ times = 20000; $ actualPath = '/tmp/actual.txt'; $ this-> generateMany ($ times, $ actualPath); $ file_content_gm = file_get_contents ($ this-> gmPath); $ file_content_actual = file_get_contents ($ actualPath); $ this-> assertTrue ($ file_content_gm == $ file_content_actual);  private function generateMany ($ times, $ fileName) ... private function generateOutput ($ seed) …

Creamos otra prueba para comparar las salidas, nos aseguramos testGenerateOutput () Solo genera la salida una vez y no hace nada más. También movimos el archivo de salida maestro de oro. "gm.txt" en el directorio actual porque "/ tmp" se puede borrar cuando el sistema se reinicia. Para nuestros resultados reales, todavía podemos usarlo. En la mayoría de los sistemas UNIX "/ tmp" está montado en la RAM, por lo que es mucho más rápido que el sistema de archivos. Si lo hiciste bien, las pruebas deberían pasar sin problema..

Es muy importante recordar marcar nuestra prueba del generador como "omitida" para futuros cambios. Si se siente más cómodo con comentar o incluso eliminarlo por completo, hágalo. Es importante que nuestro Maestro Dorado no cambie cuando cambiemos nuestro código. Se generó una vez y no queremos modificarlo nunca, para que podamos estar seguros de que nuestro código recién generado siempre se compara con el original. Si se siente más cómodo haciendo una copia de seguridad de la misma, por favor proceda.

function testGenerateOutput () $ this-> markTestSkipped (); $ veces = 20000; $ this-> generateMany ($ times, $ this-> gmPath); 

Solo marcaré la prueba como saltada. Esto pondrá nuestro resultado de prueba a "amarillo", es decir, todas las pruebas pasan, pero algunas se omiten o se marcan como incompletas.

Haciendo Nuestro Primer Cambio

Con las pruebas en su lugar, podemos comenzar a cambiar el código. En mi opinión profesional, todas las cadenas pueden mantenerse dentro de la para lazo. Tomaremos el código de la createRockQuestion () método, moverlo dentro de la para bucle, y eliminar el método por completo. Esta refactorización se llama Método en línea.

"Coloque el cuerpo del método en el cuerpo de sus interlocutores y elimine el método". ~ Martin Fowler

Hay un conjunto específico de pasos para hacer este tipo de refactorización, tal como lo definió Marting Fowler en Refactorización: Mejora del diseño del código existente:

  • Verificar que el método no sea polimórfico..
  • Encuentra todas las llamadas al método..
  • Reemplace cada llamada con el cuerpo del método..
  • Compilar y probar.
  • Eliminar la definición del método..

No tenemos subclases extendidas Juego, entonces el primer paso valida.

Solo hay un uso de nuestro método, dentro del para lazo.

función __construct () $ this-> players = array (); $ this-> places = array (0); $ this-> purses = array (0); $ this-> inPenaltyBox = array (0); $ this-> popQuestions = array (); $ this-> scienceQuestions = array (); $ this-> sportsQuestions = array (); $ this-> rockQuestions = array (); para ($ i = 0; $ i < 50; $i++)  array_push($this->PopQuestions, "Pop Question". $ i); array_push ($ this-> scienceQuestions, ("Science Question". $ i)); array_push ($ this-> sportsQuestions, ("Sports Question". $ i)); array_push ($ this-> rockQuestions, "Rock Question". $ i);  function createRockQuestion ($ index) return "Rock Question". $ índice; 

Ponemos el código de createRockQuestion () en el para bucle en el constructor. Todavía tenemos nuestro antiguo código. Ahora es el momento de ejecutar nuestra prueba.

Nuestras pruebas están pasando. Podemos borrar nuestro createRockQuestion () método.

función __construir () //… // para ($ i = 0; $ i < 50; $i++)  array_push($this->PopQuestions, "Pop Question". $ i); array_push ($ this-> scienceQuestions, ("Science Question". $ i)); array_push ($ this-> sportsQuestions, ("Sports Question". $ i)); array_push ($ this-> rockQuestions, "Rock Question". $ i);  la función isPlayable () return ($ this-> howManyPlayers ()> = 2); 

Finalmente deberíamos ejecutar nuestras pruebas de nuevo. Si perdimos una llamada a un método, fallarán.

Deberían pasar de nuevo. Felicidades Hemos terminado con nuestra primera refactorización..

Otras cuerdas a considerar

Cuerdas en los métodos. añadir() y roll () solo se usan para generarlos usando el Echoln () método. hacer preguntas() compara cadenas a las categorías. Esto parece aceptable también. currentCategory () por otro lado devuelve cadenas basadas en un número. En este método, hay muchas cadenas duplicadas. Cambiar cualquier categoría, excepto que Rock requeriría cambiar su nombre en tres lugares, solo en este método.

function currentCategory () if ($ this-> places [$ this-> currentPlayer] == 0) return "Pop";  if ($ this-> places [$ this-> currentPlayer] == 4) return "Pop";  if ($ this-> places [$ this-> currentPlayer] == 8) return "Pop";  if ($ this-> places [$ this-> currentPlayer] == 1) return "Science";  if ($ this-> places [$ this-> currentPlayer] == 5) return "Science";  if ($ this-> places [$ this-> currentPlayer] == 9) return "Science";  if ($ this-> places [$ this-> currentPlayer] == 2) return "Sports";  if ($ this-> places [$ this-> currentPlayer] == 6) return "Sports";  if ($ this-> places [$ this-> currentPlayer] == 10) return "Sports";  devolver "Rock"; 

Pienso que podemos hacerlo mejor. Podemos utilizar un método de refactorización llamado. Introducir variable local Y eliminar la duplicación. Siga estas pautas:

  • Añadir una variable con el valor deseado..
  • Encuentra todos los usos del valor..
  • Reemplaza todos los usos con la variable..
function currentCategory () $ popCategory = "Pop"; $ scienceCategory = "Science"; $ sportCategory = "Sports"; $ rockCategory = "Rock"; if ($ this-> places [$ this-> currentPlayer] == 0) return $ popCategory;  if ($ this-> places [$ this-> currentPlayer] == 4) return $ popCategory;  if ($ this-> places [$ this-> currentPlayer] == 8) return $ popCategory;  if ($ this-> places [$ this-> currentPlayer] == 1) return $ scienceCategory;  if ($ this-> places [$ this-> currentPlayer] == 5) return $ scienceCategory;  if ($ this-> places [$ this-> currentPlayer] == 9) return $ scienceCategory;  if ($ this-> places [$ this-> currentPlayer] == 2) return $ sportCategory;  if ($ this-> places [$ this-> currentPlayer] == 6) return $ sportCategory;  if ($ this-> places [$ this-> currentPlayer] == 10) return $ sportCategory;  devolver $ rockCategory; 

Esto es mucho mejor. Probablemente ya haya observado algunas mejoras futuras que podríamos hacer a nuestro código, pero solo estamos al comienzo de nuestro trabajo. Es tentador arreglar todo lo que encuentre inmediatamente, pero por favor no lo haga. Muchas veces, especialmente antes de que se entienda bien el código, los cambios tentadores pueden llevar a callejones sin salida o incluso a un código más roto. Si cree que existe la posibilidad de que olvide su idea, simplemente anótela con una nota adhesiva o cree una tarea en su software de gestión de proyectos. Ahora tenemos que continuar con nuestros problemas relacionados con cadenas.

En el resto del archivo, todas las cadenas están relacionadas con la salida, enviadas a Echoln (). Por el momento, los dejaremos intactos. Su modificación afectaría la impresión y la entrega de la lógica de nuestra aplicación. Son parte de la capa de presentación mezclada con la lógica empresarial. Nos ocuparemos de separar las diferentes preocupaciones en una lección futura.

Constantes mágicas

Las constantes mágicas son muy parecidas a las cuerdas mágicas, pero con valores. Estos valores pueden ser valores booleanos o números. Nos concentraremos principalmente en los números utilizados en Si declaraciones o regreso Declaraciones u otras expresiones. Si estos números tienen un significado poco claro, debemos extraerlos en variables o métodos.

include_once __DIR__. '/Game.php'; $ notAWinner; $ aGame = new Game (); $ aGame-> add ("Chet"); $ aGame-> add ("Pat"); $ aGame-> add ("Sue"); hacer $ aGame-> roll (rand (0, 5) + 1); if (rand (0, 9) == 7) $ notAWinner = $ aGame-> wrongAnswer ();  else $ notAWinner = $ aGame-> wasCorrectlyAnswered ();  while ($ notAWinner);

Vamos a empezar con GameRunner.php Esta vez y nuestro primer enfoque es el rodar() Método que obtiene algunos números aleatorios. Al autor anterior no le importó dar un significado a esos números. ¿Podemos? Si analizamos el código:

rand (0, 5) + 1

Devolverá un número entre uno y seis. La parte aleatoria devuelve un número entre cero y cinco al que siempre agregamos uno. Así es seguramente entre uno y seis. Ahora tenemos que considerar el contexto de nuestra aplicación. Estamos desarrollando un juego de trivia. Sabemos que hay algún tipo de tablero en el que nuestros jugadores deben moverse. Y para ello, tenemos que tirar los dados. Un dado tiene seis caras y puede producir números entre uno y seis. Eso parece una deducción razonable..

$ dice = rand (0, 5) + 1; $ aGame-> roll ($ dice);

¿No es eso agradable? Usamos nuevamente el concepto de refactorización de variables locales. Nombramos nuestra nueva variable $ dados y representa el número aleatorio generado entre uno y seis. Esto también hizo que nuestro próximo comentario pareciera realmente natural: Juego, dados de tirar.

¿Hiciste tus pruebas? No lo mencioné, pero necesitamos ejecutarlos con la mayor frecuencia posible. Si no lo has hecho, este sería un buen momento para ejecutarlos. Y deberían pasar.

Entonces, este fue un caso de nada más que simplemente intercambiar un número con una variable. Tomamos una expresión completa que representaba un número y la extraíamos en una variable. Esto puede considerarse técnicamente como un caso de Constancia Mágica, pero no un caso puro. ¿Qué pasa con nuestra próxima expresión aleatoria??

if (rand (0, 9) == 7)

Esto es más complicado. ¿Cuáles son cero, nueve y siete en esa expresión? Tal vez podamos nombrarlos. A primera vista, no tengo buenas ideas para cero y nueve, así que probemos siete. Si el número devuelto por nuestra función aleatoria es igual a siete, ingresaremos la primera rama de la Si declaración que produce una respuesta incorrecta. Así que tal vez nuestros siete podrían ser nombrados $ wrongAnswerId.

$ wrongAnswerId = 7; if (rand (0, 9) == $ wrongAnswerId) $ notAWinner = $ aGame-> wrongAnswer ();  else $ notAWinner = $ aGame-> wasCorrectlyAnswered (); 

Nuestras pruebas aún están pasando y el código es algo más expresivo. Ahora que hemos logrado nombrar nuestro número siete, pone el condicional en un contexto diferente. Podemos pensar en algunos nombres decentes para cero y nueve también. Son solo parámetros para rand (), así que las variables probablemente serán nombradas min-algo y max-algo.

$ minAnswerId = 0; $ maxAnswerId = 9; $ wrongAnswerId = 7; if (rand ($ minAnswerId, $ maxAnswerId) == $ wrongAnswerId) $ notAWinner = $ aGame-> wrongAnswer ();  else $ notAWinner = $ aGame-> wasCorrectlyAnswered (); 

Ahora eso es expresivo. Tenemos una ID de respuesta mínima, una máxima y otra para la respuesta incorrecta. Misterio resuelto.

do $ dice = rand (0, 5) + 1; $ aGame-> roll ($ dice); $ minAnswerId = 0; $ maxAnswerId = 9; $ wrongAnswerId = 7; if (rand ($ minAnswerId, $ maxAnswerId) == $ wrongAnswerId) $ notAWinner = $ aGame-> wrongAnswer ();  else $ notAWinner = $ aGame-> wasCorrectlyAnswered ();  while ($ notAWinner);

Pero note que todo el código está dentro de un hacer mientras lazo. ¿Necesitamos volver a asignar las variables de ID de respuesta cada vez? Yo creo que no. Intentemos moverlos fuera del bucle y ver si nuestras pruebas están pasando.

$ minAnswerId = 0; $ maxAnswerId = 9; $ wrongAnswerId = 7; do $ dice = rand (0, 5) + 1; $ aGame-> roll ($ dice); if (rand ($ minAnswerId, $ maxAnswerId) == $ wrongAnswerId) $ notAWinner = $ aGame-> wrongAnswer ();  else $ notAWinner = $ aGame-> wasCorrectlyAnswered ();  while ($ notAWinner);

Sí. Las pruebas pasan así también..

Es hora de cambiar a Game.php y busca también las Constantes Mágicas. Si tiene resaltado de código, seguramente tiene las constantes resaltadas en un color brillante. Los míos son azules y son bastante fáciles de detectar.

Encontrando la magia constante 50 en ese para El bucle fue bastante fácil. Y si observamos lo que hace el código, podemos descubrir que dentro de la para Bucle, los elementos son empujados a varias matrices. Así que tenemos algún tipo de listas, cada una con 50 elementos. Cada lista representa una categoría de pregunta y las variables son en realidad campos de clase definidos anteriormente como matrices.

$ this-> popQuestions = array (); $ this-> scienceQuestions = array (); $ this-> sportsQuestions = array (); $ this-> rockQuestions = array ();

Entonces, ¿qué puede representar 50? Apuesto a que ya tienes algunas ideas. Nombrar es una de las tareas más difíciles en la programación, si tiene más de una idea y no está seguro de cuál elegir, no se avergüence. También tengo varios nombres en mi cabeza y estoy evaluando posibilidades para elegir el mejor, incluso mientras escribo este párrafo. Creo que podemos ir con un nombre conservador para 50. Algo en la línea de$ preguntasEn cada Categoría o $ categorySize o algo similar.

$ categorySize = 50; para ($ i = 0; $ i < $categorySize; $i++)  array_push($this->PopQuestions, "Pop Question". $ i); array_push ($ this-> scienceQuestions, ("Science Question". $ i)); array_push ($ this-> sportsQuestions, ("Sports Question". $ i)); array_push ($ this-> rockQuestions, "Rock Question". $ i); 

Eso parece decente. Podemos conservarlo. Y las pruebas son, por supuesto, están pasando..

function isPlayable () return ($ this-> howManyPlayers ()> = 2); 

Que es dos Estoy seguro de que en este punto la respuesta es clara para usted. Eso es fácil:

function isPlayable () $ minimumNumberOfPlayers = 2; return ($ this-> howManyPlayers ()> = $ minimumNumberOfPlayers); 

¿Estás de acuerdo? Si tiene una idea mejor, no dude en comentar a continuación. ¿Y tus pruebas? ¿Todavía están pasando??

Ahora en el rodar() Método también tenemos algunos números: dos, cero, 11 y 12.

si ($ roll% 2! = 0)

Eso está bastante claro. Extraeremos esa expresión en un método, pero no en este tutorial. Todavía estamos en la fase de comprensión y búsqueda de constantes mágicas y cadenas. Entonces, ¿qué hay de 11 y 12? Están enterrados dentro del tercer nivel de Si declaraciones Es bastante difícil entender lo que representan. Tal vez si miramos las líneas a su alrededor..

if ($ this-> places [$ this-> currentPlayer]> 11) $ this-> places [$ this-> currentPlayer] = $ this-> places [$ this-> currentPlayer] - 12; 

Si el lugar o la posición del jugador actual es mayor que 11, su posición se reducirá a la actual menos menos 12. Esto suena como un caso de cuando un jugador llega al final del tablero o del campo de juego y se vuelve a colocar en su posición inicial. posición. Probablemente la posición cero. O, si nuestro tablero de juego es circular, pasar por la última posición marcada pondrá al jugador en la primera posición relativa. Así que 11 podría ser el tamaño del tablero.

$ boardSize = 11; if ($ this-> inPenaltyBox [$ this-> currentPlayer]) if ($ roll% 2! = 0) // ... // if ($ this-> places [$ this-> currentPlayer]> $ boardSize) $ this-> places [$ this-> currentPlayer] = $ this-> places [$ this-> currentPlayer] - 12;  // ... // else // ... // else // ... // if ($ this-> places [$ this-> currentPlayer]> $ boardSize) $ this-> places [$ this -> currentPlayer] = $ this-> places [$ this-> currentPlayer] - 12;  //… //

No te olvides de reemplazar 11 en ambos lugares dentro del método. Esto nos obligará a mover la asignación de variables fuera de la Si Declaraciones, justo en el primer nivel de sangría..

Pero si 11 es el tamaño del tablero, ¿cuál es 12? Restamos 12 de la posición actual del jugador, no 11. ¿Y por qué no establecemos la posición a cero en lugar de restar? Porque eso haría que nuestras pruebas fracasaran. Nuestra suposición previa de que el jugador terminará en la posición cero después del código dentro de la Si La sentencia se ejecuta, estaba equivocada. Digamos que un jugador está en la posición diez y saca un cuatro. 14 es mayor que 11, entonces la resta ocurrirá. El jugador terminará en posición. 10 + 4-12 = 2.

Esto nos lleva a otro nombre posible para 11 y 12. Creo que es más apropiado llamar a 12 $ boardSize. ¿Pero qué nos deja eso por 11? Tal vez $ lastPositionOnTheBoard? Un poco largo, pero al menos nos dice la verdad sobre la constante mágica.

$ lastPositionOnTheBoard = 11; $ boardSize = 12; if ($ this-> inPenaltyBox [$ this-> currentPlayer]) if ($ roll% 2! = 0) //… // if ($ this-> places [$ this-> currentPlayer]> $ lastPositionOnTheBoard) $ this-> places [$ this-> currentPlayer] = $ this-> places [$ this-> currentPlayer] - $ boardSize;  // ... // else // ... // else // ... // if ($ this-> places [$ this-> currentPlayer]> $ lastPositionOnTheBoard) $ this-> places [$ this -> currentPlayer] = $ this-> places [$ this-> currentPlayer] - $ boardSize;  //… //

¡Sé que sé! Hay una duplicación de código allí. Es bastante obvio, especialmente con el resto del código oculto. Pero por favor recuerden que buscábamos constantes mágicas. También habrá un tiempo para duplicar el código, pero no ahora.

Pensamientos finales

Dejé una última constante mágica en el código. ¿Puedes distinguirlo? Si miras el código final, será reemplazado, pero por supuesto eso sería hacer trampa. Buena suerte encontrándolo y gracias por leer..