Регистрация

[PHP, ООП, Совершенный код] Вы уверены, что пишете объектно-ориентированный код? (перевод)

Ответить на тему
Автор Сообщение
news_bot ®

Стаж: 6 лет 8 месяцев
Сообщений: 27286

Создавать темы news_bot ® написал(а)
20-Июл-2020 17:30
Цитировать

Мы, PHP-разработчики, горды тем, что пишем на ООП-языке (можно легко здесь заменить PHP на C#, Java или другой ООП-язык). Каждая вакансия содержит требования про знание ООП. В каждом собеседовании спрашивают что-нибудь про SOLID или трех "китов" ООП. Но когда дело доходит до дела — мы получаем просто классы, наполненные процедурами. ООП проявляется редко, обычно в коде библиотек.
Обычное веб-приложение — это классы ORM-сущностей, которые содержат данные из строки в базе данных и контроллеры(или сервисы — неважно), содержащие процедуры работы с этими данными. Объектно-ориентированное программирование — оно про объекты, которые владеют собственными данными, а не предоставляют их для обработки другому коду. Отличная иллюстрация этого — вопрос, который был задан в одном чате: "Как я могу улучшить этот код?"
private function getWorkingTimeIntervals(CarbonPeriod $businessDaysPeriod, array $timeRanges): array
{
    $workingTimeIntervals = [];
    foreach ($businessDaysPeriod as $date) {
        foreach ($timeRanges as $time) {
            $workingTimeIntervals[] = [
                'start' => Carbon::create($date->format('Y-m-d') . ' ' . $time['start']),
                'end' => Carbon::create($date->format('Y-m-d') . ' ' . $time['end'])
            ];
        }
    }
    return $workingTimeIntervals;
}
/**
* Удалить события из расписания
*
* @param array $workingTimeIntervals
* @param array $events
* @return array
*/
private function removeEventsFromWorkingTime(array $workingTimeIntervals, array $events): array
{
    foreach ($workingTimeIntervals as $n => &$interval) {
        foreach ($events as $event) {
            $period = CarbonPeriod::create($interval['start'], $interval['end']);
            if ($period->overlaps($event['start_date'], $event['end_date'])) {
                if ($interval['start'] <= $event['start_date'] && $interval['end'] <= $event['end_date']) {
                    $interval['end'] = $event['start_date'];
                } elseif ($interval['start'] >= $event['start_date'] && $interval['end'] >= $event['end_date']) {
                    $interval['start'] = $event['end_date'];
                } elseif ($interval['start'] <= $event['start_date'] && $interval['end'] >= $event['end_date']) {
                    $interval['start'] = $event['start_date'];
                    $interval['end'] = $event['end_date'];
                } else {
                    unset($workingTimeIntervals[$n]);
                }
            }
        }
    }
    return $workingTimeIntervals;
}

Этот код работает с временными интервалами. Метод удаляет множество одних интервалов(событий) из множества других(расписания), чтобы получить свободное время в расписании. Как видите, вся работа здесь вертится вокруг структуры из двух дат. И стиль работы с данными весьма распространённый — некая структура данных(здесь массив) и некий сторонний код, который с ней работает. Организуя код так, легко дублировать его, когда схожая логика понадобится в другом месте. Давайте попробуем сконцентрировать данные и код, с ними работающий, в одном месте.
Каждый раз начиная рефакторинг я проверяю наличие unit-тестов для данного кода. Рефакторинг без тестов может быть весьма болезненным. Код из примера их не имеет и его структура не даёт возможности быстро их написать (методы приватные). Поэтому будем аккуратно проверять логику после рефакторинга.
class Interval
{
    // Типизированные поля из PHP 7.4
    public DateTimeImmutable $start;
    public DateTimeImmutable $end;
}

Класс DateTimeImmutable был использован как более удобная альтернатива для работы с датами.
Первое требование к этому классу — начальная дата не должна быть больше конечной.
Намного удобнее когда объект, не удовлетворяющий требованиям, даже не может быть создан, поэтмоу конструктор — лучшее место для этой проверки.
Я почти всегда начинаю описывать требования к классу через unit-тесты. Позже будет понятно почему. Начнем с обычного конструктора:
class Interval
{
    public DateTimeImmutable $start;
    public DateTimeImmutable $end;
    public function __construct(DateTimeImmutable $start, DateTimeImmutable $end)
    {
        $this->start = $start;
        $this->end = $end;
    }
}

Теперь PHPUnit-тесты для нашего требования:
use App\Interval;
use PHPUnit\Framework\TestCase;
class IntervalTest extends TestCase
{
    private DateTimeImmutable $today;
    private DateTimeImmutable $yesterday;
    private DateTimeImmutable $tomorrow;
    protected function setUp(): void
    {
        $this->today = new DateTimeImmutable();
        $this->yesterday = $this->today->add(\DateInterval::createFromDateString("-1 day"));
        $this->tomorrow = $this->today->add(\DateInterval::createFromDateString("1 day"));
        parent::setUp();
    }
    public function testValidDates()
    {
        $interval = new Interval($this->yesterday, $this->today);
        $this->assertEquals($this->yesterday, $interval->start);
        $this->assertEquals($this->today, $interval->end);
    }
    public function testInvalidDates()
    {
        $this->expectException(\InvalidArgumentException::class);
        new Interval($this->today, $this->yesterday);
    }
}

Объекты дат для сегодняшнего, вчерашнего и завтрашнего дня помогут читабельности тестов. Первый тест, testValidDates, просто проверяет обычное создание интервала. Второй, testInvalidDates, пытается создать неправильный интервал и ожидает эксепшен. Первый тест пройдет нормально, второй свалится с ошибкой:
Failed asserting that exception of type "InvalidArgumentException" is thrown.

Теперь реализуем эту проверку:
class Interval
{
    public DateTimeImmutable $start;
    public DateTimeImmutable $end;
    public function __construct(DateTimeImmutable $start, DateTimeImmutable $end)
    {
        if ($start > $end) {
            throw new \InvalidArgumentException("Invalid date interval");
        }
        $this->start = $start;
        $this->end = $end;
    }
}

Теперь оба тесты будут зелеными. Спасибо современной системе типов в PHP, мы не обязаны проверять null и другие значения. Это сильно сокращает объем тестов. Однако, каждый раз, когда разработчик пишет тесты он обязан проверить краевые значения. Очевидным краевым значением для объекта Interval будут одинаковые даты начала и конца. Возможно такое или нет? Вот как unit-тесты помогают создавать хороший дизайн. Они задают хорошие вопросы еще до того, как мы написали настоящий код. До того как разработчик сам задаст их себе. Давайте решим, что пустые интервалы возможны, но добавим новый метод isEmpty к этому классу.
class Interval
{
    public DateTimeImmutable $start;
    public DateTimeImmutable $end;
    public function __construct(DateTimeImmutable $start, DateTimeImmutable $end)
    {
        if ($start > $end) {
            throw new \InvalidArgumentException("Invalid date interval");
        }
        $this->start = $start;
        $this->end = $end;
    }
    public function isEmpty(): bool
    {
        return $this->start->getTimestamp() == $this->end->getTimestamp();
    }
}
class IntervalTest extends TestCase
{
    //...
    public function testNonEmpty()
    {
        $interval = new Interval($this->yesterday, $this->today);
        $this->assertFalse($interval->isEmpty());
    }
    public function testEmpty()
    {
        $interval = new Interval($this->today, $this->today);
        $this->assertTrue($interval->isEmpty());
    }
}

Мы построили базовый класс для интервала дат. Он может быть использован вместо массивов ['start'=>,'end'=>], но большого смысла в этом нет. Давайте добавим знания об интервале дат в этот объект! Начальное задание было о создании списка свободных интервалов, на основе расписания и занятых слотов. Первый метод создавал интервалы, например:
какой-то день 08:00 - 12:00
какой-то день 13:00 - 17:00
следующий день 08:00 - 12:00
следующий день 13:00 - 17:00
...

Второй метод удалял занятые слоты из расписания, оставляя лишь свободные:
Занятые слоты:
какой-то день 08:00 - 09:00
какой-то день 16:00 - 17:00
следующий день 13:00 - 17:00
Результат:
какой-то день 09:00 - 12:00
какой-то день 13:00 - 16:00
следующий день 08:00 - 12:00
...

Я хочу переместить эту логику в класс Interval:
$period = CarbonPeriod::create($interval['start'], $interval['end']);
if ($period->overlaps($event['start_date'], $event['end_date'])) {
    if ($interval['start'] <= $event['start_date'] && $interval['end'] <= $event['end_date']) {
        $interval['end'] = $event['start_date'];
    } elseif ($interval['start'] >= $event['start_date'] && $interval['end'] >= $event['end_date']) {
        $interval['start'] = $event['end_date'];
    } elseif ($interval['start'] <= $event['start_date'] && $interval['end'] >= $event['end_date']) {
        $interval['start'] = $event['start_date'];
        $interval['end'] = $event['end_date'];
    } else {
        unset($workingTimeIntervals[$n]);
    }
}

Новый метод класса Interval: remove(Interval $other) будет изменять объект, удаляя из него переданный интервал. Код станет намного чище:
private function getWorkingTimeIntervals(CarbonPeriod $businessDaysPeriod, array $timeRanges): array
{
    $workingTimeIntervals = [];
    foreach ($businessDaysPeriod as $date) {
        foreach ($timeRanges as $time) {
            $workingTimeIntervals[] = new Interval(
                Carbon::create($date->format('Y-m-d') . ' ' . $time['start']),
                Carbon::create($date->format('Y-m-d') . ' ' . $time['end'])
            );
        }
    }
    return $workingTimeIntervals;
}
/**
* @param Interval[] $workingTimeIntervals
* @param Interval[] $events
* @return Interval[]
*/
private function removeEventsFromWorkingTime($workingTimeIntervals, $events): array
{
    foreach ($workingTimeIntervals as $n => $interval) {
        foreach ($events as $event) {
            $interval->remove($event);
            if ($interval->isEmpty()) {
                unset($workingTimeIntervals[$n]);
            }
        }
    }
    return $workingTimeIntervals;
}

Отлично. Время реализовать новый метод. И начнём, очевидно, с тестов! Когда разработчик начинает разрабатывать с тестов, он обязан подумать о требованиях к классу и методам и подумать хорошо. Это полезно. Проанализируем требования, рассмотрев возможные случаи.
Интервал не должен быть изменён, если $other не касается его.
class IntervalRemoveTest extends TestCase
{
    private DateTimeImmutable $minus10Days;
    private DateTimeImmutable $today;
    private DateTimeImmutable $yesterday;
    private DateTimeImmutable $tomorrow;
    private DateTimeImmutable $plus10Days;
    protected function setUp(): void
    {
        $this->today = new DateTimeImmutable();
        $this->yesterday = $this->today->sub(\DateInterval::createFromDateString("1 day"));
        $this->tomorrow = $this->today->add(\DateInterval::createFromDateString("1 day"));
        $this->minus10Days = $this->today->sub(\DateInterval::createFromDateString("10 day"));
        $this->plus10Days = $this->today->add(\DateInterval::createFromDateString("10 day"));
        parent::setUp();
    }
    public function testDifferent()
    {
        $interval = new Interval($this->minus10Days, $this->yesterday);
        $interval->remove(new Interval($this->tomorrow, $this->plus10Days));
        $this->assertEquals($this->minus10Days, $interval->start);
        $this->assertEquals($this->yesterday, $interval->end);
    }
}

Интервал, полностью покрытый переданным интервалом, должен стать пустым.
class IntervalRemoveTest extends TestCase
{
    public function testFullyCovered()
    {
        $interval = new Interval($this->yesterday, $this->tomorrow);
        $interval->remove(new Interval($this->minus10Days, $this->plus10Days));
        $this->assertTrue($interval->isEmpty());
    }
    public function testFullyCoveredWithCommonStart()
    {
        $interval = new Interval($this->yesterday, $this->tomorrow);
        $interval->remove(new Interval($this->yesterday, $this->plus10Days));
        $this->assertTrue($interval->isEmpty());
    }
    // and testFullyCoveredWithCommonEnd()
}

Следующие случай, когда переданный интервал частично перекрывает текущий:

Что если переданный интервал располагается внутри текущего?

Эмммм?! Наш интервал разделился на два! Весь дизайн нашего метода remove оказался неверным! Кстати, и изначальный код тоже содержит ошибку в этом месте:
} elseif ($interval['start'] <= $event['start_date'] && $interval['end'] >= $event['end_date']) {
        $interval['start'] = $event['start_date'];
        $interval['end'] = $event['end_date'];

Но обнаружена она будет сильно позже и хорошо если не пользователями приложения.
Мы даже не начали писать сам код, но уже нашли большую проблему в дизайне. Да, этот пример весьма прост и многие разработчики нашли бы проблему и без тестов, но для более сложных случаев найти проблему обычными умозаключениями будет весьма непросто. Вот почему тесты полезны. Они помогают писать код, часто избегая серьезных просчетов в дизайне. Некоторые разработчики говорят, что не любят писать тесты, потому что с ними код пишется медленней. Для простых тестов, вроде тех, которые мы написали вначале, это верно. Но для ситуаций посложнее, код с тестами часто написать бывает быстрее, чем без них! Тесты задают хорошие и весьма точные вопросы про дизайн вашего кода. Разработчик обязан создавать хороший дизайн.
Одним из возможных решений является создание нового класса — IntervalCollection, представляющего собой множество интервалов и операции над ними:
class Interval
{
    public DateTimeImmutable $start;
    public DateTimeImmutable $end;
    public function __construct(DateTimeImmutable $start,
                                DateTimeImmutable $end)
    {
        if ($start > $end) {
            throw new \InvalidArgumentException(
                                "Invalid date interval");
        }
        $this->start = $start;
        $this->end = $end;
    }
    public function isEmpty(): bool
    {
        return $this->start === $this->end;
    }
    /**
     * @param Interval $other
     * @return Interval[]
     */
    public function remove(Interval $other)
    {
        if ($this->start >= $other->end
                || $this->end <= $other->start) return [$this];
        if ($this->start >= $other->start
                && $this->end <= $other->end) return [];
        if ($this->start < $other->start
                && $this->end > $other->end) return [
            new Interval($this->start, $other->start),
            new Interval($other->end, $this->end),
        ];
        if ($this->start === $other->start) {
            return [new Interval($other->end, $this->end)];
        }
        return [new Interval($this->start, $other->start)];
    }
}
/** @mixin Interval[] */
class IntervalCollection extends \ArrayIterator
{
    public function diff(IntervalCollection $other)
            : IntervalCollection
    {
        /** @var Interval[] $items */
        $items = $this->getArrayCopy();
        foreach ($other as $interval) {
            $newItems = [];
            foreach ($items as $ourInterval) {
                array_push($newItems,
                ...$ourInterval->remove($interval));
            }
            $items = $newItems;
        }
        return new self($items);
    }
}

Класс IntervalCollection — это другой пример концентрации логики рядом с данными, которыми она оперирует. Не просто массив с объектами Interval, который обрабатывается некоторыми сторонними функциями, а целый класс с правильным именем и покрытый тестами.
Полный исходный код с тестами можно найти здесь — https://github.com/adelf/intervals-example. Текущее решение не очень оптимальное с точки зрения производительности, но логика его спрятана в методе IntervalCollection::diff и хорошо покрыта тестами. Если другой разработчик захочет оптимизировать его, он сможет это сделать без всякого страха. Любую ошибку в логике тесты поймают немедленно. Это второе преимущество unit-тестов.
Организация вашего кода объектами, которые по-настоящему владеют своими собственными данными помогают сильно уменьшить coupling (связанность или зацепление), что очень важно в больших проектах. Следующим этапом концентрации данных и кода будет переделка модификаторов доступа полей на private:
class Interval
{
    private DateTimeImmutable $start;
    private DateTimeImmutable $end;
    // methods
}

Это возможно сделать добавлением методов в стиле print(), которые помогут нам вытащить данные об интервале в нужном формате, но полностью закроет воможность работать с данными интервала извне. Но это уже точно тема для другой статьи.
 ===========
 Источник:
habr.com
===========
 ===========
 Автор оригинала: Adel F
===========Похожие новости: Теги для поиска: #_php, #_oop (ООП), #_sovershennyj_kod (Совершенный код), #_oop (ооп), #_php, #_php, #_oop (
ООП
), #_sovershennyj_kod (
Совершенный код
)
Профиль  ЛС 
Показать сообщения:     
Ответить на тему

Вы не можете начинать темы
Вы не можете отвечать на сообщения
Вы не можете редактировать свои сообщения
Вы не можете удалять свои сообщения
Вы не можете голосовать в опросах
Вы не можете прикреплять файлы к сообщениям
Вы не можете скачивать файлы

Текущее время: 01-Ноя 10:25
Часовой пояс: UTC + 5