Киски: Рефакторинг. Часть третья или причесываем шероховатости

http://php-and-symfony.matthiasnoback.nl/2015/07/refactoring-the-cat-api-client-part-3/
  • Перевод
  • Tutorial
imageВ первой и второй частях серии статей мы проделали немного работы по разделению того кода и тех лишних действий, которые мы понаписали в одной функции. В основном же мы имели дело с классами HttpClient и Cache, и их разными реализациями, чтобы написать тестируемый клиент для апи кисок.

Представление данных


До этого мы обращали много внимания на поведение и общую структуру кода, но забиывали про на данные, с которыми имеем дело. Сейчас у нас все является строками, включая возвращаемое значение CatApi::getRandomImage(). То есть вызывая этот метод, мы «знаем», что получим строку. Говорю «знаем», так как PHP может вернуть все — объект, ресурс, массив и т.д. Тем не менее, хоть в случае с RealCatApi::getRandomImage() мы и можем быть уверенны, что нам придет строка, так как мы явно приводим значение к ней, мы не можем точно сказать, что эта строка будет «полезна» (валидна) для того, кто вызвал этот метод: это может быть и пустая строка, строка, которая не содержит URL (типа «I am not a URL») и так далее.

class RealCatApi implements CatAPi
{
    ...

    /**
     * @return string URL of a random image
     */
    public function getRandomImage()
    {
        try {
            $responseXml = ...;
        } catch (HttpRequestFailed $exception) {
            ...
        }

        $responseElement = new \SimpleXMLElement($responseXml);

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

Чтобы сделать наш код правильнее и надежнее, хорошим решением будет сделать так, чтобы мы были уверены в том, что возвращаем корректное значение.

Первое, что мы можем сделать — проверить пост-условия нашего метода:

$responseElement = new \SimpleXMLElement($responseXml);

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

if (filter_var($url, FILTER_VALIDATE_URL) === false) {
    throw new \RuntimeException('The Cat Api did not return a valid URL');
}

return $url;

Хоть и правильно, но может плохо читаться. Будет еще хуже, если появится еще несколько функций, которым также требуются подобные проверки. Нужно как-то переиспользовать эту логику валидации. В любом случае, возвращаемое значение сейчас все еще остается такой же бесполезной строкой и будет круто, если мы заверим всех, что это действительно URL. В таком случае, уже любая часть программы, которая использует метод CatApi::getRandomImage(), будет точно знать, что это URL, в нем нет ошибок и вообще это не email какой-нибудь.

Объект-значение для URL


Вместо написания пост-условий для реализаций CatApi::getRandomImage(), мы можем написать пред-условия для URL наших картинок. Как мы удостоверимся, что URL картинки может существовать лишь в том случае, когда он валиден? Правильно, создадим еще один объект и удостоверимся, что он не создается с использованием чего-нибудь, что не является валидным адресом:

class Url
{
    private $url;

    public function __construct($url)
    {
        if (!is_string($url)) {
            throw new \InvalidArgumentException('URL was expected to be a string');
        }

        if (filter_var($url, FILTER_VALIDATE_URL) === false) {
            throw new \RuntimeException('The provided URL is invalid');
        }

        $this->url = $url;
    }
}

Такой тип объекта называется объект-значение (value-object).

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

new Url('I am not a valid URL');

// RuntimeException: "The provided URL is invalid"

Теперь любой объект класса URL точно представляет валидный адрес, другого быть не может. Можем снова поменять код нашей функции, чтобы возвращать новый объект:

$responseElement = new \SimpleXMLElement($responseXml);

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

return new Url($url);

Обычно, у value-объектов есть методы для их создания из примитивных типов и конвертирования в них, чтобы была возможность подготовить их для сохранения/загрузки, или же создания из других value-объектов. Для этого случая нам понадобятся методы fromString() и __toString(). Также, эти методы приводят к тому, что появляется возможность реализации методов параллельного создания (таких как fromParts($scheme, $host, $path, ...)) и специлаьных геттеров (host(), isSecure()..). Конечно, не стоит писать эти методы до того, как они действительно понадобятся.

class Url
{
    private $url;

    private function __construct($url)
    {
        $this->url = $url;
    }

    public static function fromString($url)
    {
        if (!is_string($url)) {
            ...
        }

        ...

        return new self($url);
    }

    public function __toString()
    {
        return $this->url;
    }
}

Теперь нужно изменить метод getRandomImage() и убедиться, что значение для картинки по-умолчанию также возвращается как URL объект:

class RealCatApi implements CatAPi
{
    ...

    /**
     * @return Url URL of a random image
     */
    public function getRandomImage()
    {
        try {
            $responseXml = ...;
        } catch (HttpRequestFailed $exception) {
            return Url::fromString('http://cdn.my-cool-website.com/default.jpg');
        }

        $responseElement = new \SimpleXMLElement($responseXml);

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

Естественно, подобные изменения отразятся в интерфейсе Cache и любом классе, который реализует его (FileCache, например) — поэтому нужно чтобы тот принимал и возвращал URL объекты:

class FileCache implements Cache
{
    ...

    public function put(Url $url)
    {
        file_put_contents($this->cacheFilePath, (string) $url);
    }

    public function get()
    {
         return Url::fromString(file_get_contents($this->cacheFilePath));
    }
}

Разбираем XML ответ


Остается изменить вот эту часть кода:

$responseElement = new \SimpleXMLElement($responseXml);

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

Честно говоря, я сам не люблю SimpleXML, но проблема здесь не в нем. Проблема здесь в том, что мы всегда ожидаем, что получим валидный ответ, содержащий корневой элемент, содержащий один элемент с названием data, который содержит как минимум один элемент images, содержащий один элемент image, у которого есть один элемент url, значение которого, как нам кажется, строка — URL адрес. В любой точке этой цепочки, предположение может быть ошибочным и это приведет к ошибке.

Наша задача на данный момент — обрабатывать эти ошибки, вместо того, чтобы PHP кидал исключения. Для этого снова определим свое исключение, которое затем будем ловить. И снова мы должны скрыть все детали о том, какие названия элементов и иерархия существует XML ответе. Такой объект, также, должен обработать любые исключения. Первым шагом будет введение простого DTO (data transfer object), представляющего сущность изображения от апи кисок.

class Image
{
    private $url;

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

    /**
     * @return string
     */
    public function url()
    {
        return $this->url;
    }
}

Видно, что этот DTO скрывает такие понятия как , и другие элементы из ответа. Мы заинтересованы лишь в URL адресе, поэтому мы делаем возможным доступ к нему через простой геттер url()

Теперь код в getRandomImage() выглядит как-то так:

$responseElement = new \SimpleXMLElement($responseXml);
$image = new Image((string) $responseElement->data->images[0]->image->url);

$url = $image->url();

Как видно, это пока не очень помогает, так как мы все еще зависим от этой цепочки XML элементов.

Вместо создания DTO напрямую, лучше будет сделать это через фабрику, которая будет знать о том, какая структура будет у XML ответа.

class ImageFromXmlResponseFactory
{
    public function fromResponse($response)
    {
        $responseElement = new \SimpleXMLElement($response);

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

        return new Image($url);
    }
}

Остается лишь внедрить инстанс ImageFromXmlResponseFactory в класс RealCatApi, это сократит код в методе RealCatApi::getRandomImage() до такого состояния:

$image = $this->imageFactory->fromResponse($responseXml);

$url = $image->url();

return Url::fromString($url);

Вынесение кода подобным образом дает возможность писать некоторые вещи лучшим образом. Маленькие классы проще тестировать. Как я и указывал ранее, наши тесты все еще тестируют, в основном, «идеальный сценарий» и большинство граничных случаев не покрыто тестами. Вот, например, что может случиться.

  1. Пустой ответ от сервера
  2. Битый XML
  3. Валидный XML, структура которого поменялась
  4. ...

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

Заключение


На этом, серия «Refactoring the Cat API client» (Киски: Рефакторинг) подходит к концу и, надеюсь, она вам понравилась. Если у вас есть предложения по улучшению — радости просим в комментарии к посту. Удачи!
  • +12
  • 9,5k
  • 5
Поделиться публикацией

Комментарии 5

    0
    Небольшой комментарий по обработке ошибок.

    Я бы предложил в таких строках
    throw new \InvalidArgumentException('URL was expected to be a string');
    


    в случае, если проверяемый параметр не является секретными данными (пользовательский пароль, например), в тексте исключения его включать. Иначе потом увидев в логе сообщение «URL was expected to be a string» не всегда ясно, что именно в тот момент времени было не так. Разумеется, это в том случае, если вы разрабатываете приложение, которое будет более-менее автономно функционировать без программиста, сидящего перед его консолью :)
      +1
      не обязательно выводить значение (вдруг это какой-то объект с циклическими ссылками?) Просто тип через gettype взять.
        0
        Нет, просто взять тип недостаточно. В логе нужно именно значение. Обычно тип объекта и наличие циклических ссылок на этапе разработки известны. В данном конкретном случае это была строка, поэтому ее достаточно было просто вывести как есть. Был бы массив, можно было бы вывести результат json_encode(). Была бы модель Laravel, возможно, достаточно было бы вывести результат работы toJson().
          +1
          ну давайте так… скоро мы все будем писать вместо этого, как-то так:

          function __construct($url : string) {
              // все остальное
          }
          


          и в логах у нас будет именно тип, а не значение.
            0
            Для меня иметь в логе только тип («string») недостаточно. Я хочу знать, какой вид URL вызвал исключение: «хттп:\\11.12.13» или «ya.ru», чтобы понять, как первый вариант вообще попал в программу и почему второй вызывал именно такое исключение.

    Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.

    Самое читаемое