Исходный код
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
из метода класса полностью переехал в методы экземпляра, так как завязан на контекст экземпляра (не может выполняться в отрыве от объекта).
Теория
- Принцип единственного знания
- Принцип единственной реализации