return;

Обсуждала Лулу стандарты кода по скайпу с разработчиком.

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

(Разработчик) Что по ретурнам?

(Лулу) Во-первых, ретурны всегда должны выделяться в отдельной строке:

(Разработчик) А во-вторых?

(Лулу) Допускается не более одной конструкции return в теле метода, функции или замыкания. Исключением из этого правила составляют только блоки try … catch в клиентском, “конечном” коде.

Хорошо, когда люди начинают спорить. Разработчик поспорил со мной, мы поболтали, я накатала статью на медиуме.

Чаще всего PHP-разработчики вставляют дополнительные ретурны по причине проверки предусловий. Зачастую количество этих предусловий достаточно много, они приобретают определенные ветвления и чаще всего метод становится невероятно жирным благодаря вот всем этим мелочным проверкам прав и правильности входных аргументов, от которых “никак” не избавится.

Что еще хуже, они сигнализируют о определенных архитектурных проблемах в коде.

(Разработчик) Смотри, вот, допустим, у меня есть следующий код, как ты избавишься от ретурнов?

– (Лулу) Я бы хотела сначала спросить, почему у нас не юзаются эксепшены, но вообще я вижу, что метод начинается с is... Вообще, в каждой if’е, в каждом наборе условий у нас находится какой-то валидатор, так что давай вынесем в отдельные переменные, тем самым наименуем эти наборы условий .

Далее выяснилось достаточно много интересного. Что на конструкцию $this->isGranted идеально бы ложилась аннотация, чтобы избавиться от миллиардов вызовов одного и того же метода; что все, что мы видим — это набор валидаторов, которые должны выбрасывать исключения, которые можно засунуть в классы и пройтись по объектам этих классов в цикле, а потом, как Лулу предполагает, обнаружится, что все же нам не понадобится булево значение от этого метода и переименовать его надо в checkAccess, а ловить эксепшены надо уже в контроллере. И что сам checkAccess лучше метод удалить, а создать отдельный класс ViewPrintableBillPreconditions, добавить его в аннотации ко всем методам, которые использовали этот метод и только тогда можно будет спокойно сказать — да, мы все сделали правильно. Это долгая история, Лулу не будет сейчас на это тратить время.

Вторая основная причина, по которым появляются дополнительные ретурны – прерывание работы метода или любое другое изменение изначального поведения метода, изменение его контракта.

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

(Разработчик) Хорошо, а бывают такие ситуации, когда необходимо выходить, т.е. завершать работу метода сразу после проверки какого-то условия, и чтобы не плодить лишние if’ы, и чтобы не городить эту тысячу классов, то проще добавить return;

(Лулу) Хм… ну, во-первых, нужно сразу же наименовать нашу проверку. Лулу придумала какую-то чепуху сначала, но потом разработчик ее поправил, предложим название isExistingInvoice:

(Разработчик) Это выглядит разумно, вот только клиентского кода, который использует этот метод, много. Поэтому это будет дублироваться, а так мы избавляемся от этого cmd+c/cmd+v.

(Лулу) Да, ты прав. А почему у нас вообще нужно делать этот пустой ретурн?

(Разработчик) Потому что обновление дат достаточно трудоемкая операция и она далеко не всегда нужна. Там просто есть кнопка “обновить даты” в пользовательском интерфейсе и в общем ну есть причины.

(Лулу) к, но знаешь, updateInvoiceDates он имеет некую двусмысленность вообще. Когда я буду у InvoiceService искать метод на обновление дат, я найду этот самый метод. Когда я читаю название этого метода, я уверена, что он выполнится, и я потеряю много времени, когда окажется, что данные по счетам не обновляются, и я сначала потеряю время, анализируя свой код, потом зайду в этот метод и обнаружу, что он не всегда выполняется.

(Разработчик) Ну, тогда ты напишешь другой метод, который принудительно обновит данные счетов.

(Лулу) Так именно updateInvoiceDates() и должен в обязательном порядке в соответствии со своим контрактом, данным ему от имени метода и моим привычкам чтения кода обновлять данные! То, что мы имеем сейчас — это не метод “обновить даты у счета”, а “запросить обновить даты у счета”. Давай так и переименуем метод. Именно переименуем, т.е. Refactor > Rename Method:

Заметили protected у updateInvoiceDates()? Теперь я, как разработчик где-то там в будущем, далеком и не особо, когда буду искать у сервиса InvoicesService метод на обновление дат, я не обнаружу метода updateInvoiceDates, потому что он защищен. Вместо него я обнаружу методы requestUpdateInvoiceDates и forceUpdateInvoiceDates, само название которых (а также отсутствие простого updateInvoiceDates) введет меня в некоторый ступор: э, а почему у нас тут два метода на одно и то же действие? Почему request, почему force, почему не просто update? И чтобы ответить на этот вопрос и сделать правильный выбор, я буду вынуждена посмотреть на тела этих методов и узнать о их реальном механизме работы.

Расшаривание знаний, а также защита кода с помощью WTF-метода является одним из самых сильных приемов по написанию устойчивого к процессам гниения и разложения кода.

Это не правило, а настойчивая рекомендация

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

Основная цель рекомендации ограничивать количество ретурнов до одного состоит не в абстрактном “красивом” коде, а в выяснении причины, по которым приходится в методе добавлять шизофрению.

Да, разработчик может добавить кучу иф-ретурнов в свою функцию, валидирующую скопом, но в процессе обсуждения выявляется, чем именно данная функция является — а именно набором предусловий, каждое из которых может иметь свое имя. Да, можно добавить просто return; в updateInvoiceDates() в метод и не париться, но как вы читали выше, наше иследование причин “а можем ли мы избавиться от ретурна” помогло нам и в гораздо более первую очередь другим, будущим разработчикам объявить об истинном поведении этого метода и как вследствии избежать неправильного его применения.

Надеюсь, от написания заметок на медиуме я не превращусь в хипстера.

Show your support

Clapping shows how much you appreciated Лулу’s story.