uk flag

Retour à la liste des articles

Exemple de refactorisation - Partie 3 - Améliorer la lisibilité du code

Dans la partie 2, j'ai réalisé les test unitaires de la classe UserManager. Je vais donc en profiter pour améliorer la lisibilité du code en toute sécurité. Voici La classe UserManager:

// UserManager.php
class UserManager
{
    private $databaseRepository;

    private $emailClient;

    public function __construct(DatabaseRepository $databaseRepository, EmailClient $emailClient)
    {
        $this->databaseRepository = $databaseRepository;
        $this->emailClient = $emailClient;
    }

    public function createUser($data)
    {
        $formattedUserData = $this->_formatUserData($data);
        $this->databaseRepository->saveUser($formattedUserData);
        $msg = 'a new user has been created';
        $this->emailClient->sendEmail('admin@example.com','A new user has been created', $msg);
    }

    private function _formatUserData(array $data)
    {
        $data['firstName'] = mb_strtolower($data['firstName']);
        $data['lastName'] = mb_strtolower($data['lastName']);
        $data['birthdate'] = $data['birthYear'] .'-'. $data['birthMonth'] .'-'. $data['birthDay'];
        unset($data['birthYear']);
        unset($data['birthMonth']);
        unset($data['birthDay']);
        return $data;
    }
}

Et voici les tests unitaires des méthodes de cette classe:

// UserManagerTest.php
class UserManagerTest extends \PHPUnit\Framework\TestCase
{
    public function testCreateUserMustCallMethodsSaveUserAndSendEmail()
    {
        $data = [
           'firstName' => 'John',
           'lastName' => 'Doe',
           'title' => 'Mr',
           'birthYear' => '1981',
           'birthMonth' => '06',
           'birthDay' => '04',
        ];

        $databaseRepositoryMock = $this->getMockBuilder(DatabaseRepository::class)->getMock();
        $emailClientMock = $this->getMockBuilder(EmailClient::class)->getMock();

        $databaseRepositoryMock->expects($this->once())->method('saveUser');
        $emailClientMock->expects($this->once())->method('sendEmail');

        $userManager = new UserManager($databaseRepositoryMock, $emailClientMock);
        $userManager->createUser($data);
    }

    public function testFormatUserData()
    {
        $data = [
           'firstName' => 'John',
           'lastName' => 'Doe',
           'title' => 'Mr',
           'birthYear' => '1981',
           'birthMonth' => '06',
           'birthDay' => '04',
        ];
        $expectedResult = [
            'firstName' => 'john',
            'lastName' => 'doe',
            'title' => 'Mr',
            'birthdate' => '1981-06-04'
        ];

        $databaseRepositoryDummy = $this->getMockBuilder(DatabaseRepository::class)->getMock();
        $emailClientDummy = $this->getMockBuilder(EmailClient::class)->getMock();

        $userManager = new UserManager($databaseRepositoryDummy, $emailClientDummy);
        $result = $this->_invokePrivateMethod($userManager, '_formatUserData', [$data]);

        $this->assertEquals($expectedResult, $result, 'the formatted data is not like expected');
    }

    private function _invokePrivateMethod($object, $method, array $params = [])
    {
        $reflectionClass = new \ReflectionClass($object);
        $method = $reflectionClass->getMethod($method);
        $method->setAccessible(true);
        return $method->invokeArgs($object, $params);
    }
}

Si l'on regarde en détail le contenu de la méthode _formatUserData et les données utilisées pour les tests, on peut s'apercevoir que la valeur title est présente dans les données à enregistrer en base (ligne 27) mais elle n'est pas modifiée par la méthode _formatUserData. Si je n'avais pas effectué un var_dump des données récoltée par la classe Api (cf partie 1), la simple lecture du code ne m'aurait pas permis d'avoir connaissance de la valeur title.

Au lieu de modifier la variable $data reçue en paramètre, je pourrais créer une nouvelle variable qui recevra toutes les données qui seront à enregistrer dans la base de données. Par une simple lecture du code, je pourrai alors savoir quelles sont les données à enregistrer dans la base:

// UserManager.php
class UserManager
{
    private $databaseRepository;

    private $emailClient;

    public function __construct(DatabaseRepository $databaseRepository, EmailClient $emailClient)
    {
        $this->databaseRepository = $databaseRepository;
        $this->emailClient = $emailClient;
    }

    public function createUser($data)
    {
        $formattedUserData = $this->_formatUserData($data);
        $this->databaseRepository->saveUser($formattedUserData);
        $msg = 'a new user has been created';
        $this->emailClient->sendEmail('admin@example.com','A new user has been created', $msg);
    }

    private function _formatUserData(array $data)
    {
        $data['firstName'] = mb_strtolower($data['firstName']);
        $data['lastName'] = mb_strtolower($data['lastName']);
        $data['birthdate'] = $data['birthYear'] .'-'. $data['birthMonth'] .'-'. $data['birthDay'];
        unset($data['birthYear']);
        unset($data['birthMonth']);
        unset($data['birthDay']);
        return $data;

        $result['firstName'] = mb_strtolower($data['firstName']);
        $result['lastName'] = mb_strtolower($data['lastName']);
        $result['birthdate'] = $data['birthYear'] .'-'. $data['birthMonth'] .'-'. $data['birthDay'];
        $result['title'] = $data['title'];
        return $result;
    }
}

Je peux exécuter mes tests pour savoir si mes modifications n'ont pas introduits de régressions:

OK (2 tests, 3 assertions)

Je vais rajouter un typage pour le paramètre de la méthode createUser: je souhaite indiquer que c'est une valeur de type array qui est attendue. Je vais également renommer la variable $data en $userData pour lui donner plus de sens:

// UserManager.php
class UserManager
{
    private $databaseRepository;

    private $emailClient;

    public function __construct(DatabaseRepository $databaseRepository, EmailClient $emailClient)
    {
        $this->databaseRepository = $databaseRepository;
        $this->emailClient = $emailClient;
    }

    public function createUser(array $userData)
    {
        $formattedUserData = $this->_formatUserData($userData);
        $this->databaseRepository->saveUser($formattedUserData);
        $msg = 'a new user has been created';
        $this->emailClient->sendEmail('admin@example.com','A new user has been created', $msg);
    }

    private function _formatUserData(array $userData)
    {
        $result['firstName'] = mb_strtolower($userData['firstName']);
        $result['lastName'] = mb_strtolower($userData['lastName']);
        $result['birthdate'] = $userData['birthYear'] .'-'. $userData['birthMonth'] .'-'. $userData['birthDay'];
        $result['title'] = $userData['title'];
        return $result;
    }
}

J'exécute mes tests à nouveau:

OK (2 tests, 3 assertions)

Parfait, tout fonctionne correctement :D. Cette dernière partie clôture les articles sur la refactorisation d'un code qui n'avait pas été développé pour être testable.

Pour conclure, si vous souhaitez en savoir plus sur le sujet, je vous conseille la lecture de deux livres de référence sur la refactorisation et la création de tests unitaires avec du code legacy: