Как делать не надо #1. Несуществующие поля трейтов

Этой статьей я открываю рубрику из практики "Как делать не надо". В ней будет код из реальных проектов, из которого я уберу все отсылки к самим проектам. И, конечно, разъяснение: почему так делать нельзя, и как бы поступить стоило.

Итак, первый пациент. Внутри трейта используются поля его наследников. То есть, что-то такое:

trait Foo
{
    public function bar(): string
    {
        return $this->field . self::CONSTANT . $this->method() . true;
    }
}

Примечание

Или даже хуже, $this->{$attribute}. Но это уже совсем ни в какие ворота не лезет, т.к. шанс схлопотать 500ю ошибку поднимается почти до 100%.

Что плохого?

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

/**
 * @mixin SomeClass
 */ 

Разумеется, только в том случае, если в SomeClass есть и $field, и CONSTANT, и method().

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

Заметка

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

Как правильно?

Использовать абстрактные методы:

trait Foo
{
    public function bar(): string
    {
        return $this->getField() . $this->getConstant() . $this->method();
    }

    abstract protected function getField(): string;
    abstract protected function getConstant(): string;
    abstract protected function method(): string;
}

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

Совет

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

Правильный вариант 2

Для адептов идей "Трейты - зло" и "Композиция - сила":

interface BarInterface
{
    public function getField(): string;
    public function getConstant(): string;
    public function method(): string;
}

readonly final class Foo
{
    public function bar(BarInterface $bar): string
    {
        return $bar->getField() . $bar->getConstant() . $bar->method();
    }
}

Здесь мы заменяем трейт на две вещи:

• Интерфейс с теми же тремя методами, что в трейте были abstract и protected
• Класс, в который мы реализацию этого интерфейса передаем, чтобы вызвать эти методы и использовать результаты их выполнения.

Совет

Результат, вроде бы, такой же, но есть нюанс. Теперь классы, реализующие BarInterface, не связаны общим кодом, что снижает показатель связанности кода (coupling) и увеличивает связанность (cohesion). Дальнейшее развитие и рефакторинг такого кода пройдут легче.