alekgrischenko · August 11, 2023 13:33 · May 8, 2020 · May 8, 2020 · May 8, 2020
diff --git a/text.md b/text.md
@@ -236,4 +236,20 @@ class User
     find_by_email(payload[:email]) || create!(payload)
   end
 end
+```
+
+# UPD
+
+1. спасибо @likeath, в последнем варианте `User` должен инжектиться в сервис, т.е. конструктор сервиса должен выглядеть так:
+
+```ruby
+module Excursion
+  module Services
+    class EnrollUser < Core::Service
+      attr_reader :model, :validator
+
+      def initialize(model: User, validator: Users::Validators::NewUserContract.new)
+        @model = model
+        @validator = validator
+      end
 ```
diff --git a/text.md b/text.md
@@ -1,3 +1,5 @@
+https://t.me/pepegramming
+
 Сегодня попался рельсовый код, в котором используются монады, сервисы и прочее. Решил сделать обзор с объяснением того, что в коде не нравится и что можно исправить.
 
 Данный разбор основан только на личном опыте и избегает попытку написать самый идеальный код на свете. К сожалению пошарить ссылку на код не могу, потому что автор попросил опубликовать анонимно.

diff --git a/text.md b/text.md
@@ -0,0 +1,237 @@
+Сегодня попался рельсовый код, в котором используются монады, сервисы и прочее. Решил сделать обзор с объяснением того, что в коде не нравится и что можно исправить.
+
+Данный разбор основан только на личном опыте и избегает попытку написать самый идеальный код на свете. К сожалению пошарить ссылку на код не могу, потому что автор попросил опубликовать анонимно.
+
+# Исходные данные
+
+Главная операция, которая вызывается из контроллера выглядит следующим образом:
+
+```ruby
+module Excursion
+  module Services
+    class JoinUser < Service
+      def initialize(params, record: ,
+        input_validator: Users::Validators::NewUser.new,
+        user_exists_check: Users::Services::CheckExists,
+        user_creator: Users::Services::Create
+      )
+        @params = params
+        @input_validator = input_validator
+        @record = record
+        @user_creator = user_creator
+        @user_exists_check = user_exists_check
+      end
+
+      def call
+        validated_params = yield input_validate
+        yield business_validate(validated_params.to_h)
+        save(validated_params.to_h)
+      end
+
+      private
+
+      attr_reader :params, :record, :input_validator, :user_creator, :user_exists_check
+
+      def input_validate
+        input_validator.call(params[:group]).to_monad
+      end
+
+      def business_validate(validated_params)
+        return Failure(:already_exists) if record.users.where(validated_params).exists?
+        Success()
+      end
+
+      def save(validated_params)
+        user_check_result = user_exists_check.new(validated_params[:email]).call
+        if user_check_result == Success(:user_not_exists)
+          user = yield user_creator.new(validated_params).call
+
+          record.users << user
+          return Success(:created)
+        end
+
+        record.users << user_check_result.value!
+
+        Success(:created)
+      end
+    end
+  end
+end
+```
+
+Два сервиса зависимостей, которые используются в этом примере выглядят так:
+
+```ruby
+module Users
+  module Service
+    class CheckExists < Service
+      def initialize(email, model: User)
+        @email = email
+        @model = model
+      end
+
+      def call
+        return Success(model.find_by(email: email)) if model.where(email: email).exists?
+        Success(:user_not_exists)
+      end
+
+      private
+
+      attr_reader :email, :model
+    end
+  end
+end
+
+module Users
+  module Services
+    class Create < Service
+      def initialize(params,
+        input_validator: Users::Validators::NewUser.new,
+        model: User
+      )
+        @params = params
+        @input_validator = input_validator
+        @model = model
+      end
+
+      def call
+        yield input_validate
+        save
+      end
+
+      private
+
+      attr_reader :params, :model, :input_validator
+
+      def input_validate
+        input_validator.call(params).to_monad
+      end
+
+      def save
+        Try() { model.create!(params) }
+      end
+    end
+  end
+end
+```
+
+
+# Проблемы которые нашел
+
+## Контекст > реализация
+
+Убрал общие слова из именования объектов, сервисов и данных. Как пример, в изначальном варианте есть строчка `save(validated_params.to_h)` (`Excursion::Services::JoinUser#call`), как оказалось - это запись пользователя на экскурсию, т.е. с точки зрения кода - это сохранение чего-то в базе, но с точки зрения логики это совершенно другая операция. Если использовать общее именование - придется каждый раз думать зачем этот код, а если использовать описание процесса - появляется контекст, в котором понять реализацию проще, чем по реализации понять контекст. Поэтому переименовал этот шаг в `enroll`.
+
+## Избыточность сервис объектов
+
+В изначальном примере можно увидеть 2 глобальные проблемы:
+1. Зависимость от кучи зависимостей
+2. Оба сервиса реализуют логику работы с базой данных (проверка существования пользователя и пользователя сохранение в базе).
+
+Большой соблазн сделать 2 отдельных сервиса и положить эту логику туда, как сделал автор оригинального примера. К сожалению, можно заметить, что такой подход приводит к усложнению бизнес логики, яркий пример - две строчки из `Excursion::Services::JoinUser#save`:
+
+```ruby
+def save(validated_params)
+  user_check_result = user_exists_check.new(validated_params[:email]).call
+  # вот тут приходится делать страшную проверку с использованием монады
+  # 
+  # так же, сама монада самоисключающий случай и вне контекста логики не понятно
+  # почему отсутствие пользователя успех
+  if user_check_result == Success(:user_not_exists)
+    # в данном случае приходится использовать DO notations во вложенном методе,
+    # таким образом появляется вложенность, что приводит к усложению data flow
+    user = yield user_creator.new(validated_params).call
+```
+
+Получается, что автор лишний раз обернул операцию с базой данных и тем самым раздул собственный код. Поэтому, кажется, что подобные методы должны находиться в моделе/репозитории. Так же, судя по неймингу, приходим к проблеме контекста, так как названия сервисов является описанием работы кода, а не описанием бизнес логики.
+
+## Общие слова
+
+В примере много общих слов, которые увеличивают конгнетивную нагрузку на человека, который этот код читает. Я говорю о `record`, `user_exists_check`, `validated_params`, `business_validate`, `input_validate`, `params` без контекста не понятно, что это такое, потому что терминология общая. В качестве примера: прочтите название двух сервисов и попробуйте ответить себе, что каждый из них делает:
+
+* `Excursion::Services::JoinUser`
+* `Excursion::Services::EnrollUser`
+
+## Стейт класса и микс параметров и зависимостей
+
+В оригинальном примере можно заметить, что конструктор каждого из классов представляет из себя микс из зависимостей и переменных. 
+```ruby
+def initialize(params, record:, # данные
+  input_validator: Users::Validators::NewUserContract.new, # зависимость
+  user_exists_check: Users::Services::CheckExists, # зависимость
+  user_creator: Users::Services::Create # зависимость
+)
+
+def initialize(email, model: User)
+  @email = email  # данные
+  @model = model # зависимость
+end
+
+def initialize(params, # данные
+  input_validator: Users::Validators::NewUserContract.new, # зависимость
+  model: User # зависимость
+)
+```
+
+В этом нет проблем до тех пор, пок не придется тестировать код и использовать тяжелособирающиеся зависимости (например rom). Так же, каждый вызов сервиса приведет к новой аллокации объектов, но это не так критично. Намного критичнее завязка локики на внутренний стейт объекта. Во первых, сервис перестает быть чистым(мутация данных приведет к багам), во вторых, поддержка кода становится сложнее, так как аргументы могут подсказать детали реализации. В качестве примера сравните два определения функций:
+
+* `def save(validated_params)`
+* `def save(record, validated_params)`
+
+В первом случае первая мысль приходящая в голову - параметры сохраняются в базе и создается новый объект модели. Во втором случае понимаешь, что сохраняются данные для конкретной записи. Проблема в том, что это не выдуманный пример. Если посмотреть на оригинал - `save` действительно использует `record` который является внутренним стейтом объекта. Но на самом деле `record` - это просито инстанс модели `excursion`.
+
+## Изоляция цепочек
+
+Распространный подход в рельсе: цепочки методов в моделе, которые сложно тестировать. Я говорю о случаях `record.users.where(payload).exists?` и подобных ему. Проблема заключается в том, что для такого кода сложно написать юнит тест, поэтому приходиться либо писать сложный мок, либо использовать интеграционное тестирование (на любой тест ходить в базу). В каждом из случаев возникают проблемы: моки сложно поддерживать и понять. А походы в базу увеличивают время прохождения тестов на минуты и даже часы.
+
+# Конечный результат
+
+В итоге, после рефакторинга, основанного на идеях описанных выше, получился код, в котором видно какие бизнес шаги происходят (валидация данных, проверка пользователя на участие в экскурсии, запись пользователя на экскурсию). Данный код не идеальны, но даже сейчас он позволяет отказаться от избыточности сервисов, добавляет бизнес контекст и может быть покрыт юнит тестами без сложной подготовки данных.
+
+```ruby
+module Excursion
+  module Services
+    class EnrollUser < Core::Service
+      attr_reader :validator
+
+      def initialize(validator: Users::Validators::NewUserContract.new)
+        @validator = validator
+      end
+
+      def call(excursion, params)
+        paylaod = yield validate_raw_data(params)
+        yield attender_of_excursion?(excursion, payload)
+
+        user = yield find_or_create_by_email(payload[:email])
+
+        Success(excursion.enroll(user))
+      end
+
+      def attender_of_excursion?(excursion, payload)
+        excursion.attender?(payload) ? Failure(:attender_of_excursion) : Success()
+      end
+
+      def find_or_create_by_email(payload)
+        Try() { User.find_or_create(payload) }.to_result
+      end
+    end
+  end
+end
+
+class Excursion
+  def attender?(payload)
+    users.where(payload).exists?
+  end
+
+  def enroll(user)
+    self.users << user
+    self
+  end
+end
+
+class User
+  def self.find_or_create(payload)
+    find_by_email(payload[:email]) || create!(payload)
+  end
+end
+```