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?
Jesús L.C. - Apuntes de un aprendiz 