Refactorizando legacy code en PHP Parte 3 – Condiciones complejas

Seguimos con la serie como refactorizar código legacy en PHP. Normalmente una de las características del código legacy es que es complejo, ya no solo porque la sintaxis sea difícil, sino porque para tratar de entenderlo hay que hacer un gran esfuerzo mental. Bucles, if imposibles, declaraciones de variables mezcladas con mil cosas más, métodos excesivamente largos.

Si la temperatura en la sala de servidores está por debajo de cinco grados y la humedad se eleva más del cincuenta por ciento, pero sigue por debajo de ochenta, la presión del aire es constante, entonces el técnico Juanito, que tiene al menos tres años de experiencia de trabajo en redes y administración de servidores debe ser notificado, tiene que despertar en la mitad de la noche, vestirse, salir, tomar su coche o llamar a un taxi si no tiene un coche, conducir hasta la oficina, entrar en el edificio, encender el aire acondicionado y esperar hasta que la temperatura aumente más de diez grados y la humedad es inferior a un veinte por ciento.

Si eres capaz de entender todo el párrafo sin releerlo eres un crack y no necesitas refactorizar nada 😛 Fuera de bromas muchas veces, en aplicaciones legacy nos encontramos con trozos de código tan complejos como este párrafo. La idea detrás de la refactorización es hacer que todo el código se entienda, intentar hacer funciones pequeñas que de un vistazo puedan ser leídas y comprendidas para a la hora de hacer un cambio este esté lo más acotado posible.

Por ejemplo podemos simplificar todo el parrafo anterior a algo como esto:

Si las condiciones ambientales presentan un riesgo, notificar a un tecnico de soporte de nivel 3 y esperar a que las condiciones ambientales vuelvan a valores normales

En esta simplificación no conocemos los detalles de cuales son las condiciones óptimas, no tenemos detalles, pero no importa. De todo esto debemos sacar en claro que tenemos que intentar que nuestro código sea simple. KISS (Keep it simple stupid).

Condicionales complejas

Vamos a tratar de intentar seguir el principio KISS a la filosofía del juego del trivial. Empezaremos por una refactorización sencilla y poco a poco aprendiendo como afrontar retos más complejos. Abrimos el archivo GameRunner.php y vamos a la linea 20.

    if (rand($minAnswerId, $maxAnswerId) == $wrongAnswerId)

Esta condición podemos transcribirlo en algo como si la respuesta es incorrecta…

The Extract Method Refactoring

Una buena manera de refactorizar es encontrar un patrón sencillo y repetible que nos permita afrontar situaciones similares. Como en el libro de Modernizing Legacy Applications in PHP
donde cada capitulo nos expone una manera “mecánica” de refactorizar vamos a comentar ahora unos pasos simplificados tomados de Improving the Design of Existing Code de Martin Fowler:

• Crear una nueva función con un buen nombre -> indicando que hace el método
• Copiar el código que queremos extraer a la función, pero no borrarlo del sitio original.
• Buscar en el código extraído las variables locales, estas serán candidatas a parámetros de la función.
• Ver si en el método extraído hay variables temporales, de ser así inicializarlas.
• Pasar al método destino las variables como parámetros
• Reemplazar el código por la llamada a la función
• Ejecutar los tests.

Quizás los pasos sean un poco complicados. Por suerte la mayoría de los IDEs hacen estos pasos de manera automática. Hace tiempo escribí un post sobre PHPStorm quizás pueda servirte de ayuda.

En una primera aproximación, así quedaría el código después de extraer el método.

.....
$minAnswerId = 0;
$maxAnswerId = 9;
$wrongAnswerId = 7;
function isWrongAnswer($minAnswerId, $maxAnswerId, $wrongAnswerId)
{
    return rand($minAnswerId, $maxAnswerId) == $wrongAnswerId;
}

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

    if (isWrongAnswer($minAnswerId, $maxAnswerId, $wrongAnswerId)) {
        $notAWinner = $aGame->wrongAnswer();
    } else {
        $notAWinner = $aGame->wasCorrectlyAnswered();
    }

} while ($notAWinner);

El problema es que se rompen los tests 😦 no es problema del código, sino de los tests, al utilizar require en la función generateOutput. Cada vez que ejecutamos los test nos damos cuenta que son pesados, tardan demasiado y no nos dejan escribir el código como nos gustaría, así que en cuanto terminemos esta fase tendremos que cambiarlos. Ejecutar 20000veces un algoritmo y comparar la salida… tenemos que mejorar.

Para solventar este escollo y poder seguir adelante, de momento vamos a optar por recubrir el código de GameRunner con una función run y modificar los tests para utilizar esta función.

Por un lado modificaremos la función generateOutput para que llame a un método run. También tendremos que hacer un include en la clase test para el fichero GameRunner

    protected function generateOutput($seed)
    {
        ob_start();
        srand($seed);
        run();
        $output = ob_get_contents();
        ob_end_clean();

        return $output;
    }

Por otro lado tenemos que hacer una función run en GameRunner.php que recubra todo el código.

function run()
{
    $notAWinner;

    $aGame = new Game();

    $aGame->add("Chet");
    $aGame->add("Pat");
    $aGame->add("Sue");

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

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

    } while ($notAWinner);
}

function isWrongAnswer($minAnswerId, $maxAnswerId, $wrongAnswerId)
{
    return rand($minAnswerId, $maxAnswerId) == $wrongAnswerId;
}

Volvemos a ejecutar los tests y comprobamos que todo sigue funcionando. Más adelante veremos como mejorar toda la estructura de carpetas y tests.

Condicionales negativos

Siempre hay que ser positivo, ver el lado bueno de las cosas, el vaso medio lleno,… por eso cuando nos ponemos a programar tambien debemos tenerlo en mente e intentar crear guardas de if positivas. Por eso vamos a hacer un pequeño cambio, ahora la función isWrongAnswer pasará a llamarse isCorrectAnswer y la guarda del if quedará así:

if (!isCorrectAnswer())

y así quedará la función:

function isCorrectAnswer()
{
    $minAnswerId = 0;
    $maxAnswerId = 9;
    $wrongAnswerId = 7;
    return rand($minAnswerId, $maxAnswerId) != $wrongAnswerId;
}

Utilizando un buen IDE no tendremos problemas en cambiar el nombre a los métodos

Hasta ahora solo hemos tocado GameRunner.php ahora vamos a por Game.php. El primer paso será buscar la palabra if, ir recorriendo todas las ocurrencias, analizándolas hasta dar con la sentencia que podemos refactorizar.

if ($roll % 2 != 0)

Y aquí esta. Puede parecer que esta expresión es sencilla, pero cuando estamos leyendo un trozo de código no podemos perder “el hilo” haciendo operaciones matemáticas. Bajo mi punto de vista es necesario tener el foco en el fragmento que estamos analizando, no sumas, restas, módulos. Así que una manera de extraer ese código es:

$this->isOdd($roll)

Lo mismo ocurre con este otro trozo de código:

if ($this->places[$this->currentPlayer] > $lastPositionOnTheBoard)

Es una pequeña comparación matemática, pero sería mejor extraerla a una función. Para ello es necesario que entendamos un poco del juego. La pregunta que debe responder el nombre del método es ¿qué? no cómo. Leyendo un poco el código de dentro del if podemos inducir que un nombre para la función del if.

if ($this->playerShouldStartANewLap($lastPositionOnTheBoard))

Si continuamos viendo el códgio vemos que podemos mejorar un poco más la función, además podemos arriesgarnos a hacer cambios que luego no funcionen porque siempre qu ehacemos un cambi ejecutamos test para comprobar que todo sigue OK.

Así que podemos modificar un poco la funcion así:

    protected function playerShouldStartANewLap()
    {
        $lastPositionOnTheBoard = 11;

        return $this->places[$this->currentPlayer] > $lastPositionOnTheBoard;
    }

Hemos refactorizado 2 ifs 🙂 estamos cogiendo carrera, así que a por el siguiente que podamos refactorizar, ya que no podemos refactorizarlos todos.

if ($this->currentPlayer == count($this->players))

Pasa a ser una función como esta shoudResetCurrentPlayer() por lo que debemos buscar todos los lugares en los que esta la condición anterior y llamar a la nueva función.

Conclusiones

Hoy hemos aprendido a refactorizar condicionales, a no perder el foco y sobre todo a ir dando pequeños pasos que nos ayudan a no tropezar.
Encontrar un buen nombre para nuestras variables y funciones es algo crucial que muchas veces dejamos de lado.

Este pos es una adaptación de este publicado en tutplus.