среда, 3 июня 2015 г.

Пример рефакторинга ruby-метода

Однажды в нашем проекте на Ruby on Rails в одном классе мне попался метод:
def self_payer
  if @paid_type == PaymentContact::TYPE_PERSONAL
    @self_payer ||= Payer.self_payer.is_developer.first || Payer.find_by_title!(SELF_PAYER)
  else
    if @business_type.title == Payer::BUSINESS_TYPE_ENTREPRENEUR
      @self_payer ||= Payer.self_payer.is_developer.first
    else
      @self_payer ||= Payer.find_by_title!(SELF_PAYER)
    end
  end
end

Здесь видно повторение Payer.self_payer.is_developer.first, которое мы вынесем в приватный метод first_developer.

Выражение @paid_type == PaymentContact::TYPE_PERSONAL использует переменную экземпляра класса @paid_type, а также выглядит громоздким, поэтому вынесем его в метод personal_payment? .

Выражение @business_type.title == Payer::BUSINESS_TYPE_ENTREPRENEUR тоже использует переменную экземпляра класса и также выглядит громоздко, выносим его в метод entrepreneur?.

Подумаем, пообщаемся с коллегами и выясним, что Payer.self_payer.is_developer.first всегда будет возвращать какой-то результат, поэтому уберем в первом if выражение: || Payer.find_by_title!(SELF_PAYER) .

Вот, что у нас получится в результате рефакторинга ruby метода:
def self_payer
  if personal_payment?
    @self_payer ||= first_developer
  else
    if entrepreneur?
      @self_payer ||= first_developer
    else
      @self_payer ||= Payer.find_by_title!(SELF_PAYER)
    end
  end
end

Поговорим с коллегами, чтобы понять суть этого метода. Оказывается, что он должен возвращать первого разработчика (first_developer), если платеж делает физ. лицо ИЛИ индивидуальный предприниматель. Исходя из этого, оставляем один if:
def self_payer
  if personal_payment? || entrepreneur?
    @self_payer ||= first_developer
  else
    @self_payer ||= Payer.find_by_title!(SELF_PAYER)
  end
end

В этом получившемся ruby методе переменная @self_payer повторяется. Поискав по классу, где этот метод расположен, мы выясняем, что эта переменная больше нигде не используется. Поэтому метод упрощается до такого вида:
def self_payer
  (personal_payment? || entrepreneur?) ? first_developer : Payer.find_by_title!(SELF_PAYER)
end

Таким образом, путем рефакторинга мы 11 строк исходного метода сократили до 3 путем пересмотра логики работы и вынесения излишне подробных действий в отдельные методы. Теперь метод self_payer читается легко: "Если платеж осуществляет ФЛ или ИП, то в качестве собственного юр. лица нужно вернуть первую попавшуюся компанию-разработчик (first_developer). В противном случае нужно найти плательщика по названию, сохраненному в константе класса (SELF_PAYER)".

Комментариев нет:

Отправить комментарий