Ответственность за получение списка размазана по разным методам


Исходный код

  def setup!
    @strategies =
      if message_from_our_bot?
        if @payload.has_key?("left_chat_participant") || @payload.has_key?("pinned_message")
          left_strategy = Strategies::LeftMember.new(@payload)
          pin_strategy = Strategies::PinMessage.new(@payload)

          left_strategy.setup!(**strategies_data)
          pin_strategy.setup!(**strategies_data)
          [left_strategy, pin_strategy]
        end
      else
        strategy_classes.map do |strategy_class|
          begin
            strategy = strategy_class.new(@payload)
            strategy.setup!(**strategies_data)
            strategy
          rescue => e
            Sentry.capture_exception(e)
            Rails.logger.error(e.class.to_s)
            Rails.logger.error(e.message)
            Rails.logger.error(e.backtrace.join("\n"))
          end
        end
      end
  end

  SERVICE_STRATEGIES = [
    Strategies::NewMember,
    Strategies::LeftMember,
    Strategies::PinMessage
  ]

  PENALTY_STRATEGIES = [
    Strategies::SpamFilter,
    Strategies::ForwardChannel,
    Strategies::StopList,
    Strategies::MediaContent,
    Strategies::Doubles,
    Strategies::ExternalLinks,
    Strategies::Dialogs,
  ]

  ALL_STRATEGIES = SERVICE_STRATEGIES + PENALTY_STRATEGIES

  def self.strategy_classes(user = nil, chat = nil)
    if user&.ta_staff? || user&.in_white_list?
      SERVICE_STRATEGIES
    elsif current_chat_subscriber?(user, chat)
      subscriber_strategies = [
        Strategies::ForwardChannel,
        Strategies::StopList,
        Strategies::Doubles,
        Strategies::Dialogs
      ]

      SERVICE_STRATEGIES + subscriber_strategies
    else
      ALL_STRATEGIES
    end
  end

  def strategy_classes
    self.class.strategy_classes(@user, @chat).select do |strategy_class|
      exclude_strategies.exclude? strategy_class.to_s
    end
  end

Что не так в этом коде

В исходном коде ответственность за формирование списка стратегий размазана между двумя методами: setup! и strategy_classes, причём одна и та же настройка стратегий выполняется двумя разными способами в методе setup!.

Вариант рефакторинга

  def setup!
    @strategies =
      strategy_classes.map do |strategy_class|
        begin
          strategy = strategy_class.new(@payload)
          strategy.setup!(**strategies_data)
          strategy
        rescue => e
          Sentry.capture_exception(e)
          Rails.logger.error(e.class.to_s)
          Rails.logger.error(e.message)
          Rails.logger.error(e.backtrace.join("\n"))
        end
      end
  end

  NEW_MEMBER_STRATEGIES = [
    Strategies::NewMember,
  ]
  LEFT_MEMBER_STRATEGIES = [
    Strategies::LeftMember,
  ]
  PINNED_STRATEGIES = [
    Strategies::PinMessage,
  ]
  SERVICE_STRATEGIES = [
  ]
  SUBSCRIBER_STRATEGIES = SERVICE_STRATEGIES + [
    Strategies::ForwardChannel,
    Strategies::StopList,
    Strategies::Doubles,
    Strategies::Dialogs,
  ]
  PENALTY_STRATEGIES = [
    Strategies::SpamFilter,
    Strategies::ForwardChannel,
    Strategies::StopList,
    Strategies::MediaContent,
    Strategies::Doubles,
    Strategies::ExternalLinks,
    Strategies::Dialogs,
  ]
  ALL_STRATEGIES = SERVICE_STRATEGIES + PENALTY_STRATEGIES

  def strategy_classes
    if @payload.has_key?("new_chat_members")
      NEW_MEMBER_STRATEGIES
    elsif @payload.has_key?("left_chat_participant")
      LEFT_MEMBER_STRATEGIES
    elsif @payload.has_key?("pinned_message")
      PINNED_STRATEGIES
    elsif @user&.ta_staff? || @user&.in_white_list?
      SERVICE_STRATEGIES #- skip_strategies # сервисные стратегии отсутствуют, поэтому в вычитании нет необходимости
    elsif current_chat_subscriber?
      SUBSCRIBER_STRATEGIES - skip_strategies
    else
      ALL_STRATEGIES - skip_strategies
    end
  end

  def skip_strategies
    Tg::UserWhiteListItem.where(tg_username: @payload.dig('from', 'username')&.downcase).pluck(:strategy).map(&:constantize)
  end

После рефакторинга метод setup! занимается только настройкой стратегий, ничего не зная о том, как формируется список стратегий, причём настройку выполняет однообразно.

Метод strategy_classes из метода класса полностью переехал в методы экземпляра, так как завязан на контекст экземпляра (не может выполняться в отрыве от объекта).

Теория

  • Принцип единственного знания
  • Принцип единственной реализации