Запрещенные изменения в коде или продолжение истории ремонта одного крана

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

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

Запрещенные изменения в коде и рецепты как их обойти

Удаление класса/интерфейса

Вместо удаления класса либо интерфейса мы отмечаем данную сущность аннотацией @deprecated . Отмечаем также все методы этой сущности как @deprecated для того, чтобы все IDE подсвечивали правильно все deprecated методы.

Причина по которой сущность стала @deprecated должна быть указана после аннотации. @see аннотация должна быть использована для рекомендации нового API, которое должно быть использовано вместо исключенного.

/** * @deprecated because new api was introduced * @see \New\Api */

Удаление public либо protected метода

Вместо удаления метода, который может служить публичным контрактом либо контрактом для наследования нужно пометить метода как @deprecated. При этом нужно сохранить контракт метода.

Добавление нового метода в класс или интерфейс

Так как Magento не знает как будет использоваться интерфейс как API или как точка расширения (SPI), то добавление нового метода в контракт также являетяс обратно несовместимым изменением (дальше — BiC), потому что если интерфейс используется как SPI, т.е. сторонние разработчики (дальше — 3PD) предоставляют свою реализацию для него, и подменяют через DI конфигурацию реализацию из коробки. Введя новый метод в контракт сущности — мы принесем проблемы для класса 3PD, у которого нет реализации для нового контракта.

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

В этом случае нужно:

Создать новый интерфейс с новым методом
Новый интерфейс может также содержать другие методы, которые содержались в оригинальном классе, если это целесообразно и ведет к увеличению cohesion
В этом случае соответствующие методы оригинального класса должны быть помечены как @deprecated с добавлением аннотации @see, которая будет указывать на контракт во вновь созданном интерфейсе.
Старые методы должны проксировать вызовы в методы созданного интерфейса вместо дублирования логики, а также для обеспечения консистентности данных при работе плагинов и ивентов
Если изменения делаются в рамках PATCH релиза, то новый интерфейс не может быть помечен как @api

Удаление статических функций

Добавление параметров, включая опциональные, в публичные методы

Исходя из того, что 3PD попрежнему могут использовать Inheritance Based API и наследовать Magento классы расширяя их контракт, то добавление параметра в метод может привести к тому, что мы поломаем класс-наследник, который также добавил опциональный параметр к изначальному контракту.

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

Добавление параметра в protected метод

Сохраняем метод как есть. Создаем новый метод с новой сигнатурой, и помечаем старый метод как @deprecated. Если возможно создаем новый метод как private.

Изменение типа принимаемых аргументов методом

Изменение типа возвращаемого значения методом

Изменение типа бросаемых исключений

*только в случае если новый тип исключительной ситуации не является подтипом старого контракта.

Изменение контракта конструктора

Для того, чтобы добавить новый параметр в конструктор необходимо сделать параметр опциональным и добавить его последним в список принимаемых аргументов.
В теле конструктора должна быть проверка передана ли новая зависимость, и если новая зависимость не была передана (значение переданного аргумента — null) — извлечь зависимость используя статический метод Magento\Framework\App\ObjectionManager::getInstance().

Пример:

class ExistingClass  {     /**       * @var \New\Dependency\Interface $newDependency      */     private $newDependency;      public function __construct(         \Old\Dependency\Intreface $oldDependency,         $oldRequiredConstructorParameter,         $oldOptinalConstructorParameter = null,         \New\Dependency\Interface $newDependency = null     ) {         ...         $this>newDependency = $newDependency ?: \Magento\Framework\App\ObjectManager::getInstance()             ->get(\New\Dependency\Interface::class);         ...     }      public function existingFunction() {         // Existing functionality             ...         // Use $this->newDependency wherever the new dependency is needed             ...     } }

Всегда ли BC фикс выглядит уродливо (как кран на КДПВ) потому что мы ограничены в стольких вещах?

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

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

Рефакторинг увеличения Coupling Between Objects и предотвращение Dependency Hell

Если же мы вносим новую валидную зависимость в класс, например, для того чтобы пофиксить баг, и после этого мы переходим порог количества допустимых внешних зависимостей (13)
Мы должны:

Сохранить обратную совместимость класса, т.е. его Публичный и Протектед интерфейсы. Но должен быть выполнен рефакторинг, который приведет к тому, что части логики класса будут вынесены в отдельные сущности — небольшие специализированные классы.
Cуществующий класс в этом случае должен выполнять роль фассада, для того чтобы обеспечить корректную работу существующих использований методов, над которыми был произведен рефакторинг.
Старые public/protected методы должны быть отмечены как @deprecated с указанием @see аннотации на вновь созданные методы в новом классе.
Все неиспользуемые private свойства класса могут быть удалены
Все неиспользуемые protected свойства класса должны быть помечены как @deprecated
Тип в PHP DocBlock неиспользуемой protected переменной свойства класса может быть удален, таким обрразом он не будет посчитан PHPMD как внешняя зависимость.
Type hinting в конструкторе для неиспользуемой зависимости должен быть удален, таким обрразом он не будет посчитан PHPMD как внешняя зависимость.
@SuppressWarnings(PHPMD.UnusedFormalParameter) должен быть добавлен для конструктора, чтобы предотвратить срабатывание статической проверки на неиспользуемые аргументы переданные методу

После выполнения действий описанных выше наш ~~код~~ кран засияет новыми красками. И что самое важное — контракт останется таким же как и был до изменений, а соответственно клиенты не заметят этих изменений!

ссылка на оригинал статьи https://habrahabr.ru/post/325124/