Refactorizando legacy code en PHP Parte 9 – Los últimos toques

Ya casi estamos acabando con la refactorización, parece mentira cuando empezamos con un solo archivo PHP, ahora tenemos clases, interfaces e incluso inversión de dependencias. Somos capaces de modificar algo sin miedo porque tenemos tests que nos indican si estamos cometiendo errores. Además podemos ampliar la funcionalidad de nuestra aplicación de una manera más o menos sencilla.

En este tutorial vamos a atacar a la “clase” GameRunner. Bueno a los métodos del archivo GameRunner para convertirlos en una clase para más adelante terminar de refactorizar los métodos que nos quedan de la clase Game.

Llegando a ser orientado a objetos

A pesar de que tenemos la mayor parte de nuestro código orientado a objetos, algunas funciones están solas en un archivo. Así que el objetivo de este post es convertir esos procedimientos en algo más orientado a objetos.

Lo primero es envolver todos los métodos en una clase a la que llamaremos RunerFunctions

const WRONG_ANSWER_ID = 7;
const MIN_ANSWER_ID = 0;
const MAX_ANSWER_ID = 9;

class RunnerFunctions {

    function run()
    {
          // ... //
    }

    function didSomeoneWin(Game $aGame, $isCorrectAnswer)
    {
          // ... //
    }

    function isCorrectAnswer($minAnswerId = MIN_ANSWER_ID, $maxAnswerId = MAX_ANSWER_ID)
    {
          // ... //
    }

}

Al modificar esta clase también tenemos que modificar los tests para utilizar la nueva clase.

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

        return $output;
    }

Pero tenemos un problema y es que al ejecutar los test (acordaos de quitar $this->markTestSkipped();) tenemos un error: Fatal error: Call to undefined function. No debemos preocuparnos, tan solo tenemos llamar a la función de manera correcta utilizando $this->…

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

        } while (!$this->didSomeoneWin($aGame, $this->isCorrectAnswer()));
    }

Tanto en la clase como en los tests

    function testCanFindWrongAnswer()
    {
        $runner = new RunnerFunctions();
        $this->assertFalse($runner->isCorrectAnswer(WRONG_ANSWER_ID, WRONG_ANSWER_ID));
    }

Una vez que tengamos los tests de nuevo en verde, debemos refactorizarlos para crear un método setUp y mover la creación del objeto allí.

 /** @var  RunnerFunctions */
    protected $runner;

    public function setUp()
    {
        $this->runner = new RunnerFunctions();
    }

Con esto evitamos la duplicidad y hacemos que nuestro código sea más legible. Ahora si nos paramos a pensar, hemos llamado a nuestra variable $runner y a nuestra clase RunnerFUunction. Quizás sería mejor tener coherencia en los nombres y renombrar la clase para que se llame Runner
Ahora tenemos un archivo Runner, pero el test se llama GameRunnerTest tambien debemos cambiar el nombre a la clase de Tests para mantener la coherencia.

Hemos llegado a una estructura como esta:

|-GameLegacy
    |- CLIDisplay
    |- Display
    |- Game
    |- Runner
|-Test
    |- GameTest
    |- RunnerTest

Incluso podríamos crear un archivo que fuese el que lanzara el juego, algo así como RunnerGame. De momento lo dejaremos como esta para centrarnos en analizar el trabajo que nos queda por afrontar y reflexionar sobre todo lo que hemos hecho hasta ahora.

Analizando las preocupaciones

Es importante se autocrítico con nosotros mismos, preguntarnos constantemente si ¿las variables tienen un buen nombre?, si ¿el nombre de método esta bien escogido?, si ¿la responsabilidad de la clase es única?… Estas pequeñas cuestiones nos harán mejorar nuestro código y sorprendentemente harán que otros desarrolladores también lo entiendan.

Llegados a este punto de la refactorización debemos empezar a preguntarnos si los métodos tienen buenos nombres, no solo eso, sino si están en la clase correcta. Por ejemplo en la clase runner tenemos un método llamado isCOrrectAnswer, quizás no esté en el lugar más indicado o quizás el nombre del método no sea el correcto. Ahora que tenemos cobertura de tests y que conocemos nuestro dominio del problema (el juego del Trivial) podemos afrontar estas preocupaciones.

También tenemos constantes definidas en la clase Runner, estas constantes son esenciales para Game, así que lo mejor será moverlas.

class Game
{
    const WRONG_ANSWER_ID = 7;
    const MIN_ANSWER_ID = 0;
    const MAX_ANSWER_ID = 9;

Una vez movidas también debemos hacer algunos cambios para que los tests vuelvan a funcionar, pero nada más lejos de nombrar correctamente a las constantes.

De la misma manera todavía tenemos métodos un poco largos que debemos refactorizar.

Atacando a los métodos largos

El método wasCorrectlyAnswered será nuestra primera victima. COmo hemos aprendido antes lo primero va a ser tener una serie de tests. Esto puede resultar a veces algo difícil. En nuestro caso tenemos varios ifelse en los que cada camino de código devuelve un valor.

    public function testWasCorrectlyAnsweredAndGettingOutOfPenaltyBoxWhileBeingAWinner()
    {
        $this->setAPlayerInPenaltyBox();
        $this->_game->isGettingOutOfPenaltyBox = true;
        $this->setCurrentPlayerAWinner();
        $this->assertTrue($this->_game->wasCorrectlyAnswered());
    }

    protected function setCurrentPlayerAWinner()
    {
        $this->_game->purses[$this->_game->currentPlayer] = Game::$numberOfScoreToWin;
    }

Hemos elegido el primer camino del if. Es más incluso reutilizamos otros métodos que utilizamos en otros tests de GameTest.

    public function testWasCorrectlyAnsweredAndGettingOutOfPenaltyBoxWhileNOTBeingAWinner()
    {
        $this->setAPlayerInPenaltyBox();
        $this->setCurrentPlayerNotAWinner();
        $this->assertTrue($this->_game->wasCorrectlyAnswered());
    }

    protected function setCurrentPlayerNotAWinner()
    {
        $this->_game->purses[$this->_game->currentPlayer] = 0;
    }

También podemos crear métodos adicionales para ayudarnos en los tests. Ahora solo quedan 3 pruebas más para el resto de casos.

Cambiando el nombre y trabajando con el método wasCorrectlyAnswered

Lo que más choca del método es la variable $winner en la que su nombre no termina de cuadrar, quizás sería mejor $notAWinner. Para ello tan solo tenemos que utilizar las utilidades del IDE para renombrar la variable.

Si analizamos un poco el método vemos que hay muchas pares repetidas, así lo que haremos será unir todas las partes comunes en una función.

    protected function selectNextPlayer()
    {
        $this->currentPlayer++;
        if ($this->shoudResetCurrentPlayer()) {
            $this->currentPlayer = 0;
        }
    }

    protected function getCorrectlyAnsweredForAPlayer()
    {
        $this->giveCurrentUserACoin();
        $this->display->playerCoins(
            $this->players[$this->currentPlayer],
            $this->purses[$this->currentPlayer]
        );

        $notAWinner = $this->didNotPlayerWin();
        $this->selectNextPlayer();

        return $notAWinner;
    }

        protected function getCorrectlyAnsweredForPlayersInPenaltyBox()
    {
        if ($this->isGettingOutOfPenaltyBox) {
            $this->display->correctAnswer();

            return $this->getCorrectlyAnsweredForAPlayer();
        } else {
            return $this->getCorrectlyAnsweredForPlayerStayingInPenaltyBox();
        }
    }

Al final el código de la función wasCorrectlyAnswered queda reducido a algo como esto

function wasCorrectlyAnswered()
    {
        if ($this->inPenaltyBox[$this->currentPlayer]) {
            return $this->getCorrectlyAnsweredForPlayersInPenaltyBox();
        }

        return $this->getCorrectlyAnsweredForPlayersNotInPenaltyBox();

    }

Lo mejor de todo esto es poder hacer cambios sin miedo ya que podemos modificar refactorizar como queramos porque después ejecutamos los tests y vemos el resultado. De la misma manera lo mejor es usar un buen IDE que puede realizar la extracción a funciones de manera automática.

Como regla general , lo mejor es tener if pequeños de no más de una linea

Lo ideal es tener métodos pequeños de no más de 20 lineas que puedan leerse de un vistazo porque los nombres de las funciones son fáciles de entender, donde los nombres de las funciones solo hablan de funcionalidad no dan detalles de implementación.

¿Qué pasa con el método wrongAnswer

Este método no es tan grande solo tiene 11 lineas, pero comparado con el resto quizás habría que refactorizarlo. El ser humano tiene una capacidad para entender limitada, así que para entender fácilmente los métodos lo mejor hacer su lógica lo más sencilla posible intentando abstraer todos los problemas.

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

        $this->currentPlayer++;
        if ($this->shoudResetCurrentPlayer()) {
            $this->currentPlayer = 0;
        }

        return true;
    }

Si vemos este método, tenemos algo de funcionalidad extraída en funciones como selectNextPlayer

Y ya que estamos continuar extrayendo la a una función $this->inPenaltyBox[$this->currentPlayer] = true; quedando finalmente el método completo así:

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

        return true;
    }

Se podría refactorizar incluso un poco más, pero sería complicar un método que parece sencillo de entender. La idea de la refactorización es dejar un código entendible, testeado y testeable.

Algunas estadísticas

Podemos ver algunas estadísticas si incluimos alguna librería como esta a composer.

phploc 2.1-g83fc223 by Sebastian Bergmann.

Size
  Lines of Code (LOC)                              435
  Comment Lines of Code (CLOC)                       0 (0.00%)
  Non-Comment Lines of Code (NCLOC)                435 (100.00%)
  Logical Lines of Code (LLOC)                     145 (33.33%)
    Classes                                        140 (96.55%)
      Average Class Length                          35
        Minimum Class Length                        11
        Maximum Class Length                        85
      Average Method Length                          2
        Minimum Method Length                        1
        Maximum Method Length                       14
    Functions                                        0 (0.00%)
      Average Function Length                        0
    Not in classes or functions                      5 (3.45%)

Cyclomatic Complexity
  Average Complexity per LLOC                     0.15
  Average Complexity per Class                    6.50
    Minimum Class Complexity                      1.00
    Maximum Class Complexity                     16.00
  Average Complexity per Method                   1.49
    Minimum Method Complexity                     1.00
    Maximum Method Complexity                    10.00

Dependencies
  Global Accesses                                    0
    Global Constants                                 0 (0.00%)
    Global Variables                                 0 (0.00%)
    Super-Global Variables                           0 (0.00%)
  Attribute Accesses                                94
    Non-Static                                      92 (97.87%)
    Static                                           2 (2.13%)
  Method Calls                                      65
    Non-Static                                      65 (100.00%)
    Static                                           0 (0.00%)

Structure
  Namespaces                                         1
  Interfaces                                         1
  Traits                                             0
  Classes                                            3
    Abstract Classes                                 0 (0.00%)
    Concrete Classes                                 3 (100.00%)
  Methods                                           57
    Scope
      Non-Static Methods                            57 (100.00%)
      Static Methods                                 0 (0.00%)
    Visibility
      Public Methods                                37 (64.91%)
      Non-Public Methods                            20 (35.09%)
  Functions                                          0
    Named Functions                                  0 (0.00%)
    Anonymous Functions                              0 (0.00%)
  Constants                                          3
    Global Constants                                 0 (0.00%)
    Class Constants                                  3 (100.00%)

Hemos terminado

Bien, parece que la serie de post ha llegado al final. tenemos todos nuestros métodos refactorizados y tenemos test que nos han ayudado ha hacerlo.

Este post está basado en http://code.tutsplus.com/tutorials/refactoring-legacy-code-part-11-the-end–cms-22476

Conclusiones

Comenzamos la serie de post como un juego (y nunca mejor dicho) teníamos un código muy feo y poco entendible del que no sabíamos nada. Ahora tenemos un código con test, una arquitectura que nos permite añadir nuevas funcionalidades y test para asegurarnos de que todos nuestro código esta bajo control.

No solo hemos refactorizado una aplicación de juguete, sino hemos aprendido a afrontar y reto y terminarlo con todo el aprendizaje que ello conlleva.
Empezamos atacando lateralmente al problema, usando el ingenio para crear test con los que poder hacer pequeños cambios. Poco a poco fuimos pensando más en la arquitectura, entendimos los principios SOLID y los aplicamos.

Hemos llegado a tener clases con responsabilidades separadas, Mocks para evitar dependencias en los tests y finalmente después de todo el esfuerzo tenemos un código legible y entendible.

Nuestras funciones son pequeñas, con unos nombres bien pensados y las variables son identificadas. De un vistazo leyendo el nombre del método sabemos lo que hace.

Espero que esta serie de posts os hallan gustado ¿ que os han parecido? ¿queréis más series como esta con nuevos problemas? ¿Alguna sugerencia?

Anuncios

Comenta la entrada

Introduce tus datos o haz clic en un icono para iniciar sesión:

Logo de WordPress.com

Estás comentando usando tu cuenta de WordPress.com. Cerrar sesión / Cambiar )

Imagen de Twitter

Estás comentando usando tu cuenta de Twitter. Cerrar sesión / Cambiar )

Foto de Facebook

Estás comentando usando tu cuenta de Facebook. Cerrar sesión / Cambiar )

Google+ photo

Estás comentando usando tu cuenta de Google+. Cerrar sesión / Cambiar )

Conectando a %s