Киски: Рефакторинг

Добрый день, Хабровчане!

Продолжаю совмещать развитие навыков перевода c английского и изучение интересных, с моей точки зрения, материалов по программированию и делюсь с вами слегка адаптированным переводом первой части из небольшого цикла статей про рефакторинг от голландца Matthias Noback, который живет в городе Зейст что вблизи Утрехта.

По большей части, в трех статьях речь идет о рефакторинге, а именно о выделении отдельных сущностей и создании самостоятельных частей кода, которые затем удобно тестировать и модифицировать. Это, естественно, приведет к увеличению количества абстракций и усложнению всей задачи, но Noback без этого не может.

Смысловое значение этой статьи не затронуто, а все изменения (заголовок и пара замен названия сервиса) сделаны лишь с целью разбавить довольную сухую и, надеюсь, простую в плане сложности материала статью, чтобы затем перейти к следующим частям всего цикла.

Итак, для начала, вот вам кусочек кода:

class CatApi
{
    public function getRandomImage()
    {
        if (!file_exists(__DIR__ . '/../../cache/random') 
            || time() - filemtime(__DIR__ . '/../../cache/random') > 3) {
            $responseXml = @file_get_contents(
                'http://thecatapi.com/api/images/get?format=xml&type=jpg'
            );
            if (!$responseXml) {
                // апи кисок упало или что-нибудь в этом роде
                return 'http://cdn.my-cool-website.com/default.jpg';
            }

            $responseElement = new \SimpleXMLElement($responseXml);

            file_put_contents(
                __DIR__ . '/../../cache/random',
                (string)$responseElement->data->images[0]->image->url
            );

            return (string)$responseElement->data->images[0]->image->url;
        } else {
            return file_get_contents(__DIR__ . '/../../cache/random');
        }
    }
}

Как вы уже могли понять, функция getRandomImage() возвращает случайный адрес картинки из апи кисок (да, оно существует!). Если функция вызывается несколько раз в течение 3 секунд — возвращается такой же результат как и ранее, чтобы предотвратить сильную нагрузку на API и, соответственно, медленные ответы.

Если бы этот код работал по принципу «один раз запустил и получил ответ», то было бы не так важно, каков он. Но так как все это закончится реальным приложением, которое работает в продакшене — придется порефакторить.

Проблемы, которые я здесь вижу:

1. Одна функция решает две задачи — получение случайного адреса и его кеширование
2. В коде есть низкоуровневые детали, такие как пути до файлов, адреса, жизненный цикл кэша, имена XML — и все это захардкожено, что в свою очередь влияет на понимание того, что же, собственно, происходит.

Все вместе это уже само по себе является проблемой — взглянем, хотя бы, на «юнит-тест» для всего этого:

class CatApiTest extends \PHPUnit_Framework_TestCase
{
    protected function tearDown()
    {
        @unlink(__DIR__ . '/../../cache/random');
    }

    /** @test */
    public function it_fetches_a_random_url_of_a_cat_gif()
    {
        $catApi = new CatApi();

        $url = $catApi->getRandomImage();

        $this->assertTrue(filter_var($url, FILTER_VALIDATE_URL) !== false);
    }

    /** @test */
    public function it_caches_a_random_cat_gif_url_for_3_seconds()
    {
        $catApi = new CatApi();

        $firstUrl = $catApi->getRandomImage();
        sleep(2);
        $secondUrl = $catApi->getRandomImage();
        sleep(2);
        $thirdUrl = $catApi->getRandomImage();

        $this->assertSame($firstUrl, $secondUrl);
        $this->assertNotSame($secondUrl, $thirdUrl);
    }
}

Что плохого? Для тестирования кэша нам нужна пара вызовов sleep. Также, мы проверяем что возвращаемое значение является валидным адресом (при этом вовсе нет проверки того, что значение является адресом апи кисок).

Попробуем немного улучшить ситуацию

Разделяем подходы — кеширование и реальные запросы

Когда смотришь на код выше, то становится понятно, что кеширование переплетается с получением новой картинки. Но это даже не так плохо, как может показаться. Ведь обычно код для подобного сценария примерно такой:

if (!isCachedRandomImageFresh()) {
    $url = fetchFreshRandomImage();

    putRandomImageInCache($url);

    return $url;
}

return cachedRandomImage();

Без разницы, загружается ли что-то в «ленивом» режиме (lazy-load) или же оно снова генерируется каждые Х секунд — этот шаблон в том или ином виде будет присутствовать в коде.

Разделив эти две вещи на разные части, у нас появится возможность тестировать получение новой картинки отдельно от её кеширования. Нет смысла знать о деталях кеширования, когда основная задача — проверить, что код действительно получает разные картинки.

Сперва мы вынесем весь код, относящийся к кэшированию в новый класс с таким же интерфейсом:

class CachedCatApi
{
    public function getRandomImage()
    {
        $cacheFilePath = __DIR__ . '/../../cache/random';
        if (!file_exists($cacheFilePath) 
            || time() - filemtime($cacheFilePath) > 3) {

            // киска не кэширована - значит нужна новая
            $realCatApi = new CatApi();
            $url = $realCatApi->getRandomImage();

            file_put_contents($cacheFilePath, $url);

            return $url;
        }

        return file_get_contents($cacheFilePath);
    }
}

Плюс, я изменил некоторые оплошности и теперь код читается лучше:

1. Убрал все лишние упоминания __DIR__. '/../../cache/random'
2. Убрал блок с else — он не нужен вообще, так как if всегда что-нибудь возвращает.
3. Класс CatApi теперь содержит только логику запроса к API:

class CatApi
{
    public function getRandomImage()
    {
        $responseXml = @file_get_contents('http://thecatapi.com/api/images/get?format=xml&type=jpg');
        if (!$responseXml) {
            // апи кисок упало или что-нибудь в этом роде
            return 'http://cdn.my-cool-website.com/default.jpg';
        }

        $responseElement = new \SimpleXMLElement($responseXml);

        return (string)$responseElement->data->images[0]->image->url;
    }
}

Инверсия зависимостей: введение абстракций

Уже сейчас можно разделить тест, потому что у нас есть два отдельных класса, но есть один момент: мы будем вынуждены тестировать класс CatApi дважды, так как CachedCatApi все еще использует его напрямую. Можно сделать большой шаг к улучшению — ввести абстракцию для CatApi, чтобы подменять его, когда у нас не будет желания делать настоящие HTTP запросы во время тестирования кэширующего поведения CachedCatApi.

Для этого выделим отдельный интерфейс для двух классов, благо оба этих класса используют один и тот публичный интерфейс (имеется в виду то, что у двух разных на данный момент классов есть один и тот же публичный метод getRandomImage()). Назовем его CatApi, а старый класс переименуем в RealCatApi.

interface CatApi
{
    /**
     * @return string
     */
    public function getRandomImage();
}

class RealCatApi implements CatApi
{
    ...
}

class CachedCatApi implements CatApi
{
    ...
}

Теперь нужно запретить CachedCatApi создавать объекты CatApi. Вместо этого сделаем возможность использовать его для получения новой, случайной киски. Такой подход называется внедрением зависимости: мы предоставляем объект с необходимыми требованиями.

class CachedCatApi implements CatApi
{
    private $realCatApi;

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

    public function getRandomImage()
    {
        ...

            $url = $this->realCatApi->getRandomImage();

        ...
    }
}

Теперь мы действительно можем разделить юнит тесты на два отдельных:

class CachedCatApiTest extends \PHPUnit_Framework_TestCase
{
    protected function tearDown()
    {
        @unlink(__DIR__ . '/../../cache/random');
    }

    /** @test */
    public function it_caches_a_random_cat_gif_url_for_3_seconds()
    {
        $realCatApi = $this->getMock('RealCatApi');
        $realCatApi
            ->expects($this->any())
            ->will($this->returnValue(
                // реальный апи всегда возвращает случайный адрес
                'http://cat-api/random-image/' . uniqid()
            );

        $cachedCatApi = new CachedCatApi($realCatApi);

        $firstUrl = $cachedCatApi->getRandomImage();
        sleep(2);
        $secondUrl = $cachedCatApi->getRandomImage();
        sleep(2);
        $thirdUrl = $cachedCatApi->getRandomImage();

        $this->assertSame($firstUrl, $secondUrl);
        $this->assertNotSame($secondUrl, $thirdUrl);
    }
}

class RealCatApiTest extends \PHPUnit_Framework_TestCase
{
    /** @test */
    public function it_fetches_a_random_url_of_a_cat_gif()
    {
        $catApi = new RealCatApi();

        $url = $catApi->getRandomImage();

        $this->assertTrue(filter_var($url, FILTER_VALIDATE_URL) !== false);
    }
}

Заключение

Чего мы добились? Теперь мы можем тестировать кеширование отдельно от обычного поведения — получения случайной картинки от API. Это значит, что два этих сценария могут развиваться отдельно друг от друга, может быть даже в совершенно разных направлениях, при этом сохраняя общую работоспособность, так как они придерживаются одного контракта (интерфейса).

Что осталось сделать? Вообще, много чего. Все еще нет возможности тестировать RealCatApi в обход создания реальных HTTP запросов (что снижает надежность такого теста и портит само понятие такого теста как «юнит-тест»). Аналогичная ситуация с CachedCatApi — он пытается кешировать в файловую систему, что опять же, мы не хотим делать в юнит-тесте, так как это медленно и влияет на глобальное состояние.

Во второй части будет рассказано о том, как избавиться от зависимостей файловой системы и выполнения реальных HTTP запросов. Ну а затем Matthias Noback поиграется с URL и XML.

UPD: Киски: Рефакторинг. Часть вторая или лечение зависимостей