Refactorizando servicio después del testing

Hace unos días empezamos con la kata TripServiceKata y conseguimos hacer tests unitarios de la clase TripService, pero ¿de verdad vamos a conformarnos con eso? Hemos conseguido tener test utilizando una clase recubrimiento, ahora vamos a refactorizar el proyecto TripService para que no sea necesario utilizar recubrimientos.

Utilizaremos dependencia de inyección para facilitarnos el trabajo a la hora de probar, he incluso aprenderemos a utilizar mocks.

¿Cómo dejamos el código?

Ahora podemos hacer cambios de una manera sencilla y sin miedo a equivocarnos demasiado, ya que con los tests veremos si algo ha dejado de funcionar. El primer paso será hacer un poco más sencillo el código intentando lanzar la excepción lo antes posible. Así que crearemos una función llamada isUserLogged que lanzará una excepción si la variable es null

    private function isUserLogged($loggedUser)
    {
        if ($loggedUser === null) {
            throw new UserNotLoggedInException();
        }
    }

Vemos que es un paso sencillo y que con los tests es aun más fácil, pero ¿por qué hemos empezado por aquí? Dásicamente saber por dónde empezar una refactorización es una cuestión de práctica. Hay una serie de «malos olores»(bad smells) que nos hacen decantarnos por un sitio u otro. Al final todo es cuestión de práctica, por eso hacer katas es una buena manera de practicar. Una primera aproximación, para ver las partes de código a las que atacar podría ser «object calisthenic».

Funciones pequeñas

La mejor manera de refactorizar es intentar hacer funciones pequeñas, que tengan entradas y salidas bien definidas, con ello conseguimos aislar el problema a una sola función y centrar nuestros esfuerzos en mejorarla.

Después de hacer este cambio debemos seguir teniendo todos los tests en verde. La manera más sencilla de refactorizar es hacer commit pequeños con los tests en verde, por ello cada vez que hagamos un cambio debemos commitear, por si llegamos a un callejón sin salida poder volver atrás sin perder nuestro trabajo.

Analizando el código podemos apreciar que estamos buscando entre los amigos de 2 usuarios para ver si realmente son amigos, parece que la responsabilidad de buscar amigos no debería estar en la clase TripService sino en la clase User, es por esto por lo que vamos a mover la responsabilidad y con ello la función.

• El primer paso es copiar la función tal y como está a la clase User

    private function areFriends(User $user, $loggedUser)
    {
        $isFriend = false;
        foreach ($user->getFriends() as $friend) {
            if ($friend == $loggedUser) {
                $isFriend = true;
                break;
            }
        }
        return $isFriend;
    }

• Modificar la función para cambiar $user por $this, con ello conseguimos eliminar el parámetro $user de la función, hacer la función publica para poder utilizarla en la clase TripService

    public function areFriends($loggedUser)
    {
        $isFriend = false;
        foreach ($this->getFriends() as $friend) {
            if ($friend == $loggedUser) {
                $isFriend = true;
                break;
            }
        }
        return $isFriend;
    }

• Cambiamos el código de la clase TripService por una llamada a esta función de la clase User

    private function areFriends(User $user, $loggedUser)
    {
        return $user->areFriends($loggedUser);
    }

• Lanzamos los tests y todo debe quedar en verde, comiteamos y después cambiamos la llamada al método areFriends(User $user, $loggedUser) por $user->areFriends($loggedUser); y lso test deben segui funcionando.

    public function getTripsByUser(User $user)
    {
        $loggedUser = $this->getLoggedUser();
        $this->isUserLogged($loggedUser);
        $isFriend = $user->areFriends($loggedUser);
        $tripList = array();
        if ($isFriend) {
            $tripList = $this->getTripList($user);
        }
        return $tripList;

    }

¿A qué el código ahora que es más compacto parece más entendible? Bueno, todavía podemos mejorarlo. En la clase User hemos dejado la función areFriends que recorre un array para buscar si un elemento está entro del mismo, el bucle for con el if puede mejorarse con

    public function areFriends($user)
    {
        return in_array($user, $this->getFriends());
    }

¿Mejor? Cuando empecé este refactor, no sabía que llegaría a algo tan simple como esto, viéndolo ahora me sorprendo de lo mucho que podemos conseguir dando pequeños pasos.

Inyección de dependencias

Como podemos ver la clase TripService no tiene constructor y además utilizamos una función UserSession::getInstance(), vamos a dar un paso paso hacer que la variable $usserSession entre a través del constructor, con ello podremos evitar el uso de esta función estática y eliminar código de nuestra clase de cobertura TripServiceCover

class TripService
{
    /** @var  UserSession */
    private $userSession;

    public function __construct($userSession)
    {
        $this->userSession = $userSession;
    }

    protected function getLoggedUser()
    {
        return $this->userSession->getLoggedUser();
    }
    ....

La clase de cobertura nos queda así:

class TripServiceKataCover extends TripService
{
    protected function getTripList(User $user)
    {
        return $user->getTrips();
    }
}

Y también tenemos que modificar los tests para pasar una instancia de UserSession, en nuestro caso pasaremos un mock así

    public function setUp()
    {
        parent::setUp();
        //Logged User
        $this->loggedUser = new User('LoggedUserName');
        $this->createNoFriend();
        $this->createFriend();
        $this->tripService = new TripServiceKataCover($this->getUserSessionMock($this->loggedUser));

    }

    private function getUserSessionMock($loggedUser)
    {
        $userSessionMock = $this->getMockBuilder('UserSession')
            ->setMethods(array('getLoggedUser'))
            ->getMock();
        $userSessionMock->method('getLoggedUser')->willReturn($loggedUser);

        return $userSessionMock;
    }

Ahora podemos dar el último paso y es pasar una instancia de TripDAO a través del constructor, con ello conseguimos poder testar de manera unitaria toda la clase TripService y además hemos aprendido a utilizar mocks

En esta serie de post hemos aprendido como refactorizar código legacy, además de utilizar mocks la lección más importante que nos debe quedar es que cuando estamos escribiendo código o refactorizando debemos tener mu claro que el código que escribimos no es para que lo entienda una máquina, sino para que pueda leerlo un compañero o tu mismo en el futuro y entenderlo sin dificultad. Dicho esto la mejor manera de mejorar es hacer katas, resolver pequeños ejercicios para entrenar nuestras habilidades como programador.