Геттеры-сеттеры и проблема с инкапсуляцией в Symfony проектах

в 5:15, , рубрики: php, symfony

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

В статьи будут рассуждения и примеры, почему такой подход опасный, а именно: нарушает нашу старую добрую инкапсуляцию, провоцирует писать код с багами и повышать сложность системы.
В статье будет опущена тема сеттеров в разного рода билдерах и тема инъекции зависимостей через сеттеры (скажу только, что не одобряем). Не будет ничего про сложные темы вроде DDD, Reach Model, про coupling/cohesion и другие умные слова — просто поговорим про инкапсуляцию. Добро пожаловать под кат.

Сразу к коду. Давайте опустим на секунду тайпхинт и указание возврата типа и подумаем, чем при работе с объектом отличается с утилитарной точки зрения такой код:

$userName = $user->name;
$user->name = $newUserName;

от кода:

$userName = $user->getName();
$user->setName($newUserName);

Опять же, если не учитывать типы, то ничем.

Cам же объект в таком случае практически ничем не отличается от обычной структуры. То есть по сути такой код провоцирует нас писать процедурный код — процедуры, которые работают с данными и меняют их.

Инкапсулция

Что же такое инкапсуляция? Под инкапсуляцией обычно понимают сокрытие данных, поведения, деталей и условий. Когда мы раскрываем детали, то наш мозг и плохое воспитание сразу толкают нас на использование всего, что доступно тут и сейчас, чтобы выполнить задачу по пути наименьшего сопротивления.

Давайте попробуем «поломать» простой объект:

class Order
{
     private const STATUS_OPEN = 'open';
     private const STATUS_DELIVERED = 'delivered';

     private ProductCollection $products;
     private string $deliveryStatus;
     private ?DateTime $deliveryDate = null;

     public function deliver()
     {
          if ($this->isDelivered()) {
               throw new OrderDomainException('Order already delivered.');
          }

          $this->deliveryDate = new DateTime();
          $this->deliveryStatus = self::STATUS_DELIVERED;
     }

     private function isDelivered(): bool
     {
         return $this->deliveryDate !== null 
                 && $this->deliveryStatus === self::STATUS_DELIVERED;
     }
}

Есть ли возможность доставить заказ «не так» где-то вне объекта? Например не указать статус или не проставить актуальную дату? Есть ли возможность начать городить логику с условием — отправлен ли заказ или нет? В данном виде нет не единой возможности. Это пример инкапсуляции — поведение и данные сокрыты, наружу торчит один лишь метод, который мы разрешаем использовать. Кроме всего прочего у данного кода хорошая внутренняя связанность (cohesion).

Теперь посмотрите на этот пример:

class Order
{
    private const STATUS_OPEN = 'open';
    private const STATUS_DELIVERED = 'delivered';

    private ProductCollection $products;
    private string $deliveryStatus;
    private ?DateTime $deliveryDate = null;

    public function getDeliveryStatus(): string
    {
        return $this->deliveryStatus;
    }

    public function setDeliveryStatus(string $deliveryStatus): void
    {
        $this->deliveryStatus = $deliveryStatus;
    }

    public function getDeliveryDate(): ?DateTime
    {
        return $this->deliveryDate;
    }

    public function setDeliveryDate(?DateTime $deliveryDate): void
    {
        $this->deliveryDate = $deliveryDate;
    }

    public function getProducts(): ProductCollection
    {
        return $this->products;
    }

    public function setProducts(ProductCollection $products): void
    {
        $this->products = $products;
    }
}

Что можно сделать с объектом такого класса? (Вопрос риторический). Условие доставлен ли заказ — где оно будет размещено, точно ли оно будет таким, точно ли оно будет одинаковым там, где нужно, чтобы оно было одинаковым? Или будет размазано и продублировано, да еще и разным? Никто вам не скажет как оно будет. Код явно говорит нам — делай как хочешь, как выйдет — посмотрим. Принцип ООП под названием инкапсуляция рухнул. По итогу этот объект будет обрабатываться в десятках процедур. А теперь опять вопрос, который выше — чем этот объект отличается от объекта с публичными полями класса в контексте работы с ним?

Сеттеры

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

Это чревато очень сложным запутанным кодом — кодом, который тяжело дебажить, а логику тяжело понимать, так как сеттинг параметров объекта происходит в неожиданных местах, слоях и с диким условиями, да еще и в отрыве от других данных.

Ну и такие объекты легко сделать не валидными. Пример доктриновской сущности:

Код некоторой сущности

/**
 * @ORMEntity
 */
class Project
{
    /**
     * @var Id
     * @ORMGeneratedValue()
     * @ORMId
     */
    private $id;
    /**
     * @var string
     * @ORMColumn(type="string")
     */
    private $name;
    /**
     * @var string
     * @ORMColumn(type="string", nullable=false)
     */
    private $status;
    /**
     * @var int
     * @ORMColumn(type="integer", nullable=true)
     */
    private $sort;
    /**
     * @var User
     * @ORMColumn(type="user_type", nullable=false)
     */
    private $user;
    /**
     * @var Department
     * @ORMOneToMany(targetEntity="Department")
     */
    private $department;
    /**
     * @var string
     * @ORMColumn(type="string", nullable=true)
     */
    private $membership;
    
    public function getId(): Id
    {
        return $this->id;
    }
    
    public function getName(): string
    {
        return $this->name;
    }
    
    public function setName(string $name): Project
    {
        $this->name = $name;
        return $this;
    }
    
    public function getStatus(): string
    {
        return $this->status;
    }
    
    public function setStatus(string $status): Project
    {
        $this->status = $status;
        return $this;
    }
    
    public function getSort(): int
    {
        return $this->sort;
    }
    
    public function setSort(int $sort): Project
    {
        $this->sort = $sort;
        return $this;
    }
    
    public function getUser(): User
    {
        return $this->user;
    }
    
    public function setUser(User $user): Project
    {
        $this->user = $user;
        return $this;
    }
    
    public function getDepartment(): Department
    {
        return $this->department;
    }
    
    public function setDepartment(Department $department): Project
    {
        $this->department = $department;
        return $this;
    }
    
    public function getMembership(): string
    {
        return $this->membership;
    }
    
    public function setMembership(string $membership): Project
    {
        $this->membership = $membership;
        return $this;
    }
}

Тут у нас есть ряд not nullable полей — указаны как явно в аннотации и без указания (по умолчанию false). Вы точно на ревью увидите, что при создании объекта и наполнении его полями — все сеттеры на not nullable были вызваны, а тесты содержат все кейсы проверок? :)
Куда лучше бы было, если бы можно было создать объект одним методом — конструктором или именованным конструктором (например статическим методом). В них был бы четко заложены зависимости и инкапсулировано необходимое для создания объекта поведение.

Новички не всегда задумываются, но Doctrine маппит данные (процесс называется гидрация) не через сеттеры, а делает это через Reflection API. Вот статья Marco Pivetta о том, как работает гидрация объектов: ocramius.github.io/blog/doctrine-orm-optimization-hydration

Геттеры

Геттеры ничем не лучше сеттеров. Они ровно также нам помогают нарушать инкапсуляцию. Конечно, тут не говорю про методы, которые нам нужны для транспорта данных (для чтения и создания DTO, для сериализации и т.д.).

Ниже приведу код, который уже поражен проблемами, которые появились из-за возможности нам «удобно» читать поля. Код из головы, но он близок к тому, что видели мои глаза и штамповали мои руки.

Представим некий домен заказа некого интернет-магазина. Допустим у нас есть хэндлер для обработки заказа по его ID, чтобы принять решение — пропускать заказ дальше через процедуры или нет:

$order = $this->orderRepository->find($orderId);
if ($order === null) {
     throw new OrderException('Order not found.');
}

if ($order->getStatus() === Order::STATUS_ACTIVE 
    && $order->getDeliveryDate() <= $date
) {
    $this->orderDeliveryService->handle($order);
}

Пару сервисов спустя проверяем город заказ и сумму бесплатной доставки по этому городу и если бесплатно — отправляем, иначе обработка продолжится дальше:

$order = $this->orderRepository->find($orderId);
$cityFreeLimit = $this->cityOrderService->getCityFreeLimit($cityName);

if ($order->getCity() === $cityName 
    && $order->getTotalPrice() > $cityFreeLimit
) {
     $delivery = 0;
     $this->orderDeliveryService->deliveryOrder($order, $delivery);
     
     return;
}

// обрабатываем заказ не с бесплатной доставкой по текущему городу

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

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

Итог

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

К сожалению факт — большое число программистов на Symfony не до конца осознают проблемы немых моделей и считают такого рода объекты лишь хранилищем данных (DAO), плодят сервисы/менеджеры. Надеюсь — доводы выше подтолкнут задуматься о проблемах процедурного кода в ООП-обертке (хочу отметить, что в данном топике нет претензий к процедурному программированию).

Ошибки и неточности готов быстро поправить, концептуально — уступок не будет :)

Приглашаю обсудить плюсы и минусы аргументов в комментариях. Если понравилось или просто одобряете — тоже прошу не проходить мимо и отметить это в комментариях.

Автор: Maksim

Источник



https://ajax.googleapis.com/ajax/libs/jquery/3.4.1/jquery.min.js