DEV Community

Bohdan Pohorilets
Bohdan Pohorilets

Posted on • Originally published at bpohoriletz.github.io on

[UA] Service Objects Глибиною В Два Рівні

#ruby #serviceobject
  • Час: 20-30 min
  • Рівень: Beginner/Intermediate
  • Код: GitHub

Перед тим як почати писати дану статтю я спробував знайти інших людейякі намагались розглянути код в даному ракурсі - знайшов наступний пост How deep is your code?

Мені потрібен був приклад складного Service Object ія знайшов його в репозиторії Discourse файл. Я в жодному разіне критикую даний код, більше того в тому ж самому репозиторії я знайшов чудовий прикладтого що я намагаюсь описати файл

Особисто я надаю перевагу маленьким класам, коротким, single purpose методам,а не одному великому шматку коду. Проте мушу визнати що даний підхідмає значний недолік - досить часто методи короткі лише тому що вони викликають інші методи, які викликають інші методи,які викликають інші методи… Отож, перш ніж я зможу побачити що насправді відбувається,мені доводиться переглядати купу методів і досить часто я вже не пам’ятаю з чого починав, колинарешті знайду те що шукав.

Я не хочу цим займатись, я не хочу переглядати 5-10 приватних методів щобмати уявлення якими будуть наслідки виклику одного публічного методу.я хочу щоб метод одразу, з першого погляду давав зрозуміти що саме відбувається,можливо є спосіб цього досягнути?

Одного разу я бачив метод оцінки складності коду, який описала Sandi Metz в своєму виступі All the Little Things - the Squint Test.Не зважаючи на те, що даний метод був застосований для оцінки вкладених conditionals, миможемо розглянути виклик методів у схожому контексті. Якщо метод викликає інший методдавайте ми зробимо відступ зліва, так ніби ми маємо вкладений if:

def parent
  child_level_two
  # some code
end

def child_level_two
  # some code
  child_level_three
end

def child_level_three
  # some code
  child_level_four
  # some code
end
Enter fullscreen mode Exit fullscreen mode

тепер давайте запишемо всі задіяні методи і зробимо відступи для кожного вкладеного виклику

def parent
  child_level_two
    child_level_three
  child_level_two
    child_level_three
    child_level_three
  child_level_two
  child_level_two
    child_level_three
    child_level_three
      child_level_four
  child_level_two
  child_level_two
    child_level_three
      child_level_four
      child_level_four
end
Enter fullscreen mode Exit fullscreen mode

ми отримали схожу фігуру, що більші нерівною є права її частина - тобільше вкладених викликів ми маємо.

КРОК #1

Зміни в коді можна подивитись тут

Service Object перед refactoring, я виніс зовнішні залежності в dependencies.rb і додав placeholders для задіяних методів

# user_updater.rb
require 'active_support'
require 'active_support/core_ext/object/blank'
require 'active_support/core_ext/time/calculations'
require 'active_model'

require_relative 'dependencies'

class UserUpdater
  delegate :change_post_owner, to: :guardian

  def update(attributes = {})
    save_options = false
    saved = nil
    old_user_name = user.name.present? ? user.name : ""

    user_profile = user.user_profile
    user_profile.location = attributes.fetch(:location) { user_profile.location }
    user_profile.dismissed_banner_key = attributes[:dismissed_banner_key] if attributes[:dismissed_banner_key].present?
    user_profile.website = format_url(attributes.fetch(:website) { user_profile.website })
    user_profile.profile_background = attributes.fetch(:profile_background) { user_profile.profile_background }
    user_profile.card_background = attributes.fetch(:card_background) { user_profile.card_background }
    if SiteSetting.enable_sso && !SiteSetting.sso_overrides_bio
      user_profile.bio_raw = attributes.fetch(:bio_raw) { user_profile.bio_raw }
    end

    user.name = attributes.fetch(:name) { user.name }
    user.locale = attributes.fetch(:locale) { user.locale }
    user.date_of_birth = attributes.fetch(:date_of_birth) { user.date_of_birth }
    if can_grant_title?(user)
      user.title = attributes.fetch(:title) { user.title }
    end

    # special handling for theme_key cause we need to bump a sequence number
    if attributes.key?(:theme_key) && user.user_option.theme_key != attributes[:theme_key]
      user.user_option.theme_key_seq += 1
    end

    OPTION_ATTR.each do |attribute|
      if attributes.key?(attribute)
        save_options = true

        if [true, false].include?(user.user_option.send(attribute))
          val = attributes[attribute].to_s == 'true'
          user.user_option.send("#{attribute}=", val)
        else
          user.user_option.send("#{attribute}=", attributes[attribute])
        end
      end
    end

    # automatically disable digests when mailing_list_mode is enabled
    user.user_option.email_digests = false if user.user_option.mailing_list_mode

    fields = attributes[:custom_fields]
    if fields.present?
      user.custom_fields = user.custom_fields.merge(fields)
    end

    User.transaction do
      if user.user_option.save && user_profile.save && user.save
        StaffActionLogger.new(@actor).log_name_change(
          user.id,
          old_user_name,
          attributes.fetch(:name) { '' }
        )

        saved = true
      end
    end

    saved
  end

  private

  attr_reader :user, :guardian

  def initialize(actor, user, guardian = Guardian.new(actor))
    @user = user
    @guardian = guardian
    @actor = actor
  end

  def format_url(website)
    return nil if website.blank?
    website =~ /^http/ ? website : "http://#{website}"
  end
end
Enter fullscreen mode Exit fullscreen mode

КРОК #2

Даний крок поділено на кілька невеликих частин:

  • #2.1 Refactor UserUpdater#update метод (commit)
  • #2.2 Refactor методи що були додані в 2.1
    • #2.2.1 Refactor UserUpdater#update_user_profile метод (commit)
    • #2.2.2 Refactor UserUpdater#update_user метод (commit)
    • #2.2.3 Refactor UserUpdater#update_user_option метод (commit)
    • #2.2.4 Refactor UserUpdater#save_user_data метод (commit)

Я пропущу деталі, тому що зміни є досить прості і звичні для refactoring, вони були зроблені щоб спроститиUserUpdater#update метод, зробити його коротким і простим для розуміння.

# user_updater.rb
class UserUpdater
  delegate :change_post_owner, to: :guardian

  def update(attributes = {})
    old_user_name = user.name.present? ? user.name : ""

    update_user_profile( user.user_profile, attributes )
    update_user( user, attributes )
    update_user_option( user.user_option, attributes )
    save_user_data(user, old_user_name, attributes)
  end

  private
  # ...
Enter fullscreen mode Exit fullscreen mode

Проте я вважаю, що тепер надто багато інформації прихованоЯкщо подивитись на тіло методу ми більше не бачимо що:

  1. В методі використовується транзакція
  2. Деякі частини змінюються лише за виконання певних умов
  3. Ми покладаємось на константу, коли визначаємо які дані змінити
  4. Не очевидно що ми повертаємо (і використовуємо значення) true/false в залежності від результату транзакції

In addition to hiding information, I storngly dislike how newОкрім прихованої інформації, мені надзвичайно не подобається як виглядаєUserUpdater#update_user_profile - це приватний метод, якийскладається лише викликів інших приватних методів і приховує існуючі conditionals.

# user_updater.rb
def update_user_profile( user_profile, attributes )
  update_geo_data( user_profile, attributes )
  update_web_data( user_profile, attributes )
  update_background_data( user_profile, attributes )
  update_bio_raw( user_profile, attributes )
end
Enter fullscreen mode Exit fullscreen mode

Я вважаю що схожі методи лише підвищують складність кодуі не мають права існувати.

КРОК #3

Зміни в коді можна подивитись тут

На цьому кроці ми позбудемось приватних методів, що були (майже) лишеобгортками навколо інших приватних методів і трохи змінимо назви методів, що залишились,щоб краще відобразити їх мету

# user_updater.rb
class UserUpdater
  delegate :change_post_owner, to: :guardian
  delegate :log_user_name_change, to: :StaffActionLogger

  def update(attributes = {})
    old_user_name = user.name.present? ? user.name : ""
    user_profile = user.user_profile
    user_option = user.user_option

    set_user_profile_geo_data( user_profile, attributes )
    set_user_profile_web_data( user_profile, attributes )
    set_user_profile_background_data( user_profile, attributes )
    set_user_profile_bio_raw( user_profile, attributes )

    set_user_bio(user, attributes, update_title: can_grant_title?(user) )
    user.custom_fields = user.custom_fields.merge( attributes.fetch( :custom_fields, {} ) )

    set_user_option_theme_key(user_option, attributes)
    OPTION_ATTR.each { |attribute| set_user_option_single_attribute(user_option, attributes, attribute) }
    # automatically disable digests when mailing_list_mode is enabled
    user_option.email_digests = false if user_option.mailing_list_mode

    return false unless User.transaction { user.user_option.save && user.user_profile.save && user.save }
    log_user_name_change( user.id, old_user_name, attributes.fetch(:name) { '' } )

    return true
  end
Enter fullscreen mode Exit fullscreen mode

UserUpdater#update метод став майже втричі більшим в порівнянні зпопередньою версією, проте тепер він значно краще розповідає свою історію.

Він не ідеальний (і ніколи таким не буде), проте ми зробимо ще один крок для його покращення і знову порівняємо з версією з Кроку #2

КРОК #4

Зміни в коді можна подивитись тут

На даному кроці ми ще трохи відшліфуємо наш код і порівняємо остаточні результати

# user_updater.rb

# Short version
class UserUpdater
  # ...
  def update(attributes = {})
    old_user_name = user.name.present? ? user.name : ""

    update_user_profile( user.user_profile, attributes )
    update_user( user, attributes )
    update_user_option( user.user_option, attributes )
    save_user_data(user, old_user_name, attributes)
  end
# Two levels deep version
class UserUpdater
  # ...
  def update(attributes = {})
    @attributes = attributes
    old_user_name = user.name.present? ? user.name : ""

    set_user_profile_geo_data
    set_user_profile_web_data
    set_user_profile_background_data
    set_user_profile_bio_raw if should_update_user_profile_bio_raw?

    set_user_bio( update_title: can_grant_title?(user) )
    user.custom_fields = user.custom_fields.merge( attributes.fetch( :custom_fields, {} ) )

    set_user_option_theme_key if should_update_user_option_theme_key?
    OPTION_ATTR.each do |attribute|
      set_user_option_single_attribute( attribute )
    end
    # automatically disable digests when mailing_list_mode is enabled
    user_option.email_digests = false if user_option.mailing_list_mode

    return false unless User.transaction { save_user_and_related_entities }
    log_user_name_change( user.id, old_user_name, attributes.fetch(:name) { '' } )

    return true
  end
Enter fullscreen mode Exit fullscreen mode

Я вважаю що довша версія має наступні переваги:

  1. Код розповідає чудову історію з великою кількістю деталей виконання
  2. Код слідує принципу Open/Closed
  3. Ви завжди щонайбільше за один крок від повного визначення будь-якого приватного методу

ПІДСУМОК

Для того щоб мати глибину рівну 2 код повинен задовольняти наступні критерії:

  1. Публічні методи не можуть викликати інші публічні методи того ж класу
  2. Приватні/Захищені методи не можуть викликати інші приватні/захищені методи

У мене є ще два не обов’ язкових правила

  1. Уникати conditionals в приватних методах
  2. Conditionals повинні використовувати приватні методи а не boolean expressions

Код:

ДЛЯ РОЗДУМІВ

  1. Дані правила досить просто виконати при написанні Service Objects, тому що вони зазвичайописуюють певний процес/алгоритм - де ще можна застосувати їх?
  2. Як щодо delegated та accessor methods, чи можна їх вважати приватними при застосуванні метрики?

Top comments (1)

Collapse
 
olesmartyniuk profile image
Oleksandr Martyniuk •

Цікава тема і особливо висновок в кінці, але мені важко було прослідкувати всі етапи рефакторингу. Вважаю, варто було б спростити код, можливо придумати якийсь синтетичний приклад. Так цінність статті виросла б. Відчувається, щоб була проведена непогана робота.

Read next

jospin6 profile image

Using Action Cable for Real-Time Applications in Ruby on Rails

Jospin Ndagano -

martinstreicher profile image

An Introduction to Nix for Ruby Developers

Martin Streicher -

carisaelam profile image

Active Record Queries Cheat Sheet

Carisa Elam -

sebyx07 profile image

Ruby2html: Streamline Your Rails Views with Pure Ruby

sebyx07 -