Чистый код. Boolean attribute

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

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

Boolean Attribute

На мой взгляд, Boolean Attribute особенный итем/тип/термин в ООП. Кажется, что он намного проще Exception, подумаешь, всего лишь два возможных значения: true и false. Но то ли я книжек в своей жизни уже перечитал, то ли опыт рефакторинга легаси проекта сказывается. Я стал замечать у себя приступы ненависти, самоуничтожения при виде, как люди могут работать с такими атрибутами.

Пример номер 1

В каком-то контроллере какого-то проекта кто-то решил изменить статус.

<?php
if ($vacancy->getStatus()) {
    $status = false;
} else {
    $status = true;
}

$vacancy->setStatus($status);
?>

Сколько претензий у вас к этому коду? Лично я затрудняюсь определить для себя, с какой стороны этот код обругать. По-моему, автор сам себя запутал. Вообще, это глобальная проблема программиста, что он не знает чего хочет и пишет код. Милый друг, остановись и подумай, прекрати смотреть и копипастить.

Пример номер два

Бытовой пример, есть некий класс с атрибутом. Если человек когда-нибудь слышал про is*, то он обязательно напишет слудующее.

<?php
class User
{
    private $isActivated = false;
    private $enabled = true;

    public function setIsActivated($isActivated)
    {
        $this->isActivated = $isActivated;
    }

    public function getIsActivated($isActivated)
    {
        return $this->isActivated;
    }

    /*
    public function isIsActivated($isActivated)
    {
        return $this->isActivated;
    }

    public function isActivated($isActivated)
    {
        return $this->isActivated;
    }
    */

    public function setEnabled($enabled)
    {
        $this->enabled = $enabled;
    }

    public function getEnabled($enabled)
    {
        return $this->enabled;
    }

    /*
    public function isEnabled($enabled)
    {
        return $this->enabled;
    }
    */
}

Вообще всевозможных вариаций трудно представить, в зависимости от конекста они вызывают разное "фи".

Это же php

Этот параграф специально для тех, кто любит поржать над php. Как вы знаете и понимаете, создав setEnabled, вы можете вызвать $user->setEnabled('Ha-ha'); $user->setEnabled(1); $user->setEnabled(0);. Немного неприятно.

Скрытые проблемы

На самом деле проблема лежит не в самом классе, а там где его используют. Программисты порой создают булевые атрибуты в качестве индикаторов чего-то. И зачастую вызов стандартных set*, get* превращает код в какую-то паутину. Вы не сразу вспоминаете или понимаете, а что на самом деле здесь происходит. Это не является какой-то причиной появления гавнокода, но это один из шагов к нему. Трудно превести конкретный пример, искать/описывать лень, но если представить...

Если мы завели какой-то флаг, значит он для чего-то нужен. Скорее всего, это будет какой-то if, а возможно и не один. Соответственно нам не нужен в большинстве случаев get*. Самое худшее, если этот флаг влияет на поведение другого атрибута этого объекта. Вызов казалось бы обычного setDeleted может привести к невалидности соседнего атрибута. А пихать дополнительную логику в этот сеттер - еще хуже.

Лекарство, но не пуля

Лично я предпочитаю следующие правила. Атрибуты делать причастиями: enabled, deleted, activated. Вместо get* использовать is*: isEnabled, isDeleted, isActivated. Вместо set* создавать методы-глаголы действия: enable, disable, delete, restore, activate, deactivate.

В случаях, когда все-таки ваш фреймворк требует set* метод, использовать что-то вроде этого:

<?php
public function enable()
{
    $this->enabled = true;
}

public function disable()
{
    $this->enabled = false;
}

public function setEnabled($enabled)
{
    if ($enabled) {
        $this->enable();
    } else {
        $this->disable();
    }
}
?>

Проблемы с наименованием

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

Всегда можно однозначно дать определение состояния, в котором находится объект при данном значении атрибута. Соответсвенно есть и название переходов из одного состояния в другое.

Если же состояние определяется сразу несколькими значениями, его формулировка занимает два-три предложения, навряд ли предложенная схема вам подойдет и поможет. Скорее всего, она запутает код еще больше. Лучше всего задуматься о пересмотре этого куска кода, попробовать вынести логику в отдельный resolver.

Комментарии