Типы функций: функции-мошенники

January 26, 2021

Начало темы здесь, здесь и здесь.

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

Как я уже писал в прошлой статье, эффекты - это то ради чего пишутся программы, и тот критерий, по которому программы оцениваются. Однако если не соблюдать определённую "эффективную гигиену", то эффекты выходят из-под контроля, что приводит к появлению багов. "Эффективная гигиена" заключается в разделении сложной логики и эффектов и выделении их в разные функции. Если же этого не делать, то появляются функции-мошенники.

Функции-мошенники бывают двух типов:

  • откровенные жулики, которые врут прямо в глаза;
  • "честный бизнес", который пишет всю правду, но мелким шрифтом.

Откровенные жулики

Под функцией-жуликом я понимаю функцию, один из эффектов которой не очевиден из её названия и/или сигнатуры. Кому не очевиден - вопрос. Пока пусть будет мне:)

Хрестоматийным примером функции-жулика является ленивая загрузка в ОРМах. По сигнатуре - рид онли функция, даже просто свойство. Смотришь в код - всё так. Пускаешь в прод - проблема N+1.

Или ещё пример:

public String decrypt(String encrypted);

Казалось бы должна быть функция без эффектов. Ан нет, это метод интерфейса. Одна из реализаций, которого идёт в сеть. А вызывается она внутри транзакции. Если сервис расшифровки вешается, то у вас условно 25 потоков обработки запросов сжирают весь пул из 25 подключений в ожидании ответа по сети и привет (да, я знаю, что повторяюсь, в книге исправлюсь:)).

Или более простой, но не менее опасный пример:

public Double calculateAverage(List<Integer> data);

Казалось бы - чистая функция, которая считает среднее значение элементов списка. Ан нет, она там внутри удаляет из списка нулевые элементы. И чем это аукнется коду, который переиспользовал (написанному другим автором или тем же автором в другое время) эту функцию по сигнатуре (ява доки-то никто не пишет, а если пишут - не читают) - не известно.

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

override suspend fun update(trxLog: TrxLog, newLog: TrxLog, newDb: InternalDb) {
   if (this.trxLog != trxLog) {
       throw ConcurrentModificationException("Concurrent transactions isn't supported yet")
   }
   storage.overwrite(Namespace("refs")["head"], newLog.hash.bytes)
   this.trxLog = newLog
   this.db = newDb
}

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

Но у этой функции есть ряд характеристик легализующих её:

  1. из её названия очевидно, что она будет обладать эффектами (в частности по отсутствию типа возврата что в Котлине примерно соответствует void-у);
  2. это единственное что делает эта функция, соотвественно все три эффекта очевидны;
  3. эти эффекты должны быть атомарны - в файл пишется ссылка на последнюю запись в журнал транзакций, в trxLog обновляется предстваление этого журнала в памяти, в db обновляется индекс этого журнала. Если хоть что-то из этого не сделать, то пользователь увидет неконсистентное или недурабельное состояние базы данных.

Для того чтобы не подкладывать самому себе бомбы в своём коде у меня всё тот же набивший оскомину рецепт - разделять эффекты и логику.

Автор лучшей книге по архитектуре причисляет функции-жулики к анти-паттернам и предостерегает от "захоронения сокровищ не подходящих местах":

Anti-pattern: Buried treasure. While most of the other advice is on what you should do, this is advice on what to avoid. It is easy to subvert the other good practices by burying treasure in inappropriate places. Responsibility-driven design asks you to allocate responsibilities to parts of your design, but you should avoid hinting at one thing while doing another (Wirfs-Brock, Wilkerson and Wiener, 1990). For example, most developers reading source code will assume that a getX() method will have no side effects and a method named launchSpaceShuttle() will do the obvious thing. If you signal to the reader that you have allocated a responsibility, you should follow through in the details.

— George Fairbanks, Just Enough Software Architecture

И это единственный анти-паттерн в своём разделе (о стиле кодирования, раскрывающем архитектуру приложения).

"Честный бизнес"

"Честный бизнес" - это функции которые одновременно:

  1. имеют эффекты;
  2. имеют высокую сложность.

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

Этим функциям посвящены главы книг и собственный анти-паттерн.

Пока что лучшее, что мне удалось найти в качестве метрики "ушлости" таких функций - это цикломатическая сложность. Но для того чтобы создать по настоящему большую проблему, одной цикломатической сложности мало. Если у вас есть чистая функция с высокой цикломатической сложностью, с ней всё равно ещё можно работать. Во-первых, поток данных в ней будет довольно простой (по определению чистой функции → отсутствия изменяемых переменных). Во-вторых, вы можете легко обложить её юнит-тестами.

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

А наблюдаемые эффекты ещё больше усугубляют проблему. Во-первых, с ними в принципе тесты писать сложнее. Во-вторых, в отличие от параметров, которых редко бывает больше трёх, эффектов в функцию можно заталкать сколько угодно и это не будет никому резать глаз. Кто-то вообще эффекты не считает, а те кто считает не разглядит их за горой if-ов и for-ов. Далее, эти эффекты могут быть в том числе чтением неявных параметров. И чтобы безопасно менять такую функцию, вам надо проверить все комбинации параметров (явных параметров) и эффектов (неявных параметров). Что на практике невозможно, конечно же (в контексте функции с высокой цикломатической сложность и большим количеством эффектов).

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

Пример функции-"честного бизнеса" я решил не приводить - вы бы его всё равно не поняли (по определению функции-"честного бизнеса") 🤣. Но если у вас был опыт работы со "зрелым" проектом или вы возвращались к своему диплому через пару лет, вы понимаете о чём я говорю:)

Как бороться с "честным бизнесом" в вашем коде? Думаю вы уже знаете🤣. Ну а если ещё вдруг нет, то - по средствам разделения сложной логики и эффектов в разные функции.

Заметка в бок.

Вообще это кажись должно было пойти в предыдущий пост, но пришло в этот:)

Проблему тестирования эффективных функций усугубляет тот факт, что в одну функцию можно затолкать сколько угодно эффектов. А в силу того, что в случае эффектов их порядок (и наличие вообще) имеет значение, нельзя написать тест кейс с одним ассертом на один эффект - в каждом тест кейсе надо проверять порядок и наличие/отсутствие всех потенциальных эффектов. Проверять все эффекты важно потому, что если функция, например, сначала удаляет строку (в реляционной БД), а потом вставляет новую с тем же первичным ключом, то нельзя написать тесты отдельно на удаление и на вставку. Каждый из этих тестов по отдельности ничего не говорит о том, работает ли функция правильно.

Заключение

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