Как я могу СУШИТЬ этот код и очистить мою модель?

Question

У меня есть следующие два метода для модели Number.

def track
  number = sanitize(tracking)

  case determine_type(number)
  when 'UPS'
    tracker = ups.track(:tracking_number => number)
    self.carrier              = Carrier.where(:name => 'UPS').first
    self.service              = tracker.service_type
    self.destination_country  = tracker.destination_country
    self.destination_state    = tracker.destination_state
    self.destination_city     = tracker.destination_city
    self.origin_country       = tracker.origin_country
    self.origin_state         = tracker.origin_state
    self.origin_city          = tracker.origin_city
    self.signature            = tracker.signature_name
    self.scheduled_delivery   = tracker.scheduled_delivery_date
    self.weight               = tracker.weight

    tracker.events.each do |event|
      new_event             = Event.new
      new_event.number      = self
      new_event.city        = event.city
      new_event.state       = event.state
      new_event.postalcode  = event.postal_code if event.postal_code
      new_event.country     = event.country
      new_event.status      = event.name
      new_event.status_code = event.type
      new_event.occured_at  = event.occurred_at

      new_event.save
    end
  end

  save
end

def update_number

  case determine_type(number)
  when 'UPS'
    tracker = ups.track(:tracking_number => tracking)
    self.carrier              = Carrier.where(:name => 'UPS').first
    self.service              = tracker.service_type
    self.destination_country  = tracker.destination_country
    self.destination_state    = tracker.destination_state
    self.destination_city     = tracker.destination_city
    self.origin_country       = tracker.origin_country
    self.origin_state         = tracker.origin_state
    self.origin_city          = tracker.origin_city
    self.signature            = tracker.signature_name
    self.scheduled_delivery   = tracker.scheduled_delivery_date
    self.weight               = tracker.weight

    last_event = self.events.ordered.first.occured_at

    tracker.events.each do |event|
      if last_event and (event.occurred_at > last_event)
        new_event             = Event.new
        new_event.number      = self
        new_event.city        = event.city
        new_event.state       = event.state
        new_event.postalcode  = event.postal_code if event.postal_code
        new_event.country     = event.country
        new_event.status      = event.name
        new_event.status_code = event.type
        new_event.occured_at  = event.occurred_at

        new_event.save
      end
    end
  end
  save
end

Как видите, большая часть кода дублируется. И проблема становится, когда я начинаю добавлять дюжину или около того других перевозчиков (FedEx, USPS, DHL и т. Д.) ... моя модель Number становится большой и волосатой.

Единственная реальная разница между track и update_number заключается в том, что update_number как сравнение условий вокруг событий, чтобы проверить, являются ли события от носителя более свежими, чем последние события, которые я сохранил в базе данных, используя эта if last_event and (event.occurred_at > last_event) строка.

Так как мне очистить этот код, чтобы моя модель не стала такой толстой?

Answer 1

Несколько вещей, которые я бы предложил:

1. Взгляните на шаблон стратегии , хотя в Ruby нет интерфейсов, но он даст вам некоторые идеикак лучше спроектировать эту функциональность (в Google для Pattern Pattern for Ruby можно найти альтернативы).По сути, вы хотели бы иметь отдельные классы для обработки ваших операторов регистра переключателя и вызова соответствующего во время выполнения.
2. Попробуйте отделить код обработки трекера от кода обработки событий.Например, я бы рассмотрел перемещение логики создания / сохранения событий для каждого из событий трекера в отдельный класс (или даже в сущность Tracker, если он у вас есть).

Надеюсь, это поможет.

Answer 2

Это должно быть решено с использованием шаблона стратегии.Для каждого возможного трекера (DHL, UPS, ...), который будет обрабатывать создание и обновление соответствующим образом.

Так что это будет выглядеть примерно так:

class Number

  def track
    tracker = get_tracker(tracking)
    tracker.create_tracking(self)
    save
  end

  def update_number
    tracker = get_tracker(tracking)
    tracker.update_tracking(self)
    save
  end

  def get_tracker(tracking)
    tracking_number = sanitize(tracking)
    case determine_type(tracking_number)
    when 'UPS'
      UPSTracker.new(tracking_number)
    when 'DHL'
      DHLTracker.new(tracking_number)
    end
  end      
end

class UPSTracker

  def initialize(tracking_number)
    @tracking_number = tracking_number
  end

  def create_tracking(number)
    tracker = ups.track(:tracking_number => number)
    update_number_properties(number, tracking)

    # store events
    tracker.events.each do |event|
      create_new_event(event)
    end
  end

  def update_tracking(number)
    tracker = ups.track(:tracking_number => number)
    update_number_properties(number, tracking)

    last_event = self.events.ordered.first.occured_at

    tracker.events.each do |event|
      if last_event and (event.occurred_at > last_event)
        create_new_event(event)
      end
    end
  end

  protected

  def update_number_properties
    number.carrier = Carrier.where(:name => 'UPS').first
    number.service              = tracker.service_type
    number.destination_country  = tracker.destination_country
    number.destination_state    = tracker.destination_state
    number.destination_city     = tracker.destination_city
    number.origin_country       = tracker.origin_country
    number.origin_state         = tracker.origin_state
    number.origin_city          = tracker.origin_city
    number.signature            = tracker.signature_name
    number.scheduled_delivery   = tracker.scheduled_delivery_date
    number.weight               = tracker.weight
  end

  def create_new_event
    new_event             = Event.new
    new_event.number      = self
    new_event.city        = event.city
    new_event.state       = event.state
    new_event.postalcode  = event.postal_code if event.postal_code
    new_event.country     = event.country
    new_event.status      = event.name
    new_event.status_code = event.type
    new_event.occured_at  = event.occurred_at
    new_event.save
  end
end

Этот код может быть улучшенЯ предполагаю, что создание событий и трекинг будет распространяться на разных носителях.Так что общий базовый класс может быть.Во-вторых, в двух методах внутри Number мы вызываем get_tracker: вероятно, этот трекер может быть выбран по времени создания (экземпляра Number) и должен быть выбран один раз и сохранен в переменной экземпляра.

Кроме того, я хотел бы добавить, что имена должны быть значимыми, поэтому класс Number не кажется мне достаточно значимым.Имя должно выражать намерение и предпочтительно совпадать с именами и понятиями из вашей проблемной области.

Answer 3

Что-то в вашем коде не имеет смысла для меня, организация немного странная, и у меня недостаточно контекста о том, как выглядит ваше приложение.ООП-способ добиться чего-то подобного в рельсах довольно прост;для этого вы также можете свернуть свою собственную стратегию, адаптер или даже шаблон Builder.

Но есть некоторые слабые результаты, вы можете реорганизовать свой код, чтобы общие части были менее навязчивыми - этонемного лучше, но create_events все еще очень case 'y:

def track
  create_events
end

def update_number
  create_events {|e| last_event and (event.occurred_at > last_event) }
end

def create_events(&block)
  case determine_type(number)
  when 'UPS'
    tracker = ups.track(:tracking_number => tracking)
    self.carrier = Carrier.where(:name => 'UPS').first
    self.assign_tracker(tracker)
  end
  tracker.events.each do |e|
    self.create_event(e) unless (block_given? && !block.call(e))
  end
  save
end

def assign_tracker(tracker)
  self.service              = tracker.service_type
  self.destination_country  = tracker.destination_country
  self.destination_state    = tracker.destination_state
  self.destination_city     = tracker.destination_city
  self.origin_country       = tracker.origin_country
  self.origin_state         = tracker.origin_state
  self.origin_city          = tracker.origin_city
  self.signature            = tracker.signature_name
  self.scheduled_delivery   = tracker.scheduled_delivery_date
  self.weight               = tracker.weight
end

def create_event(event)
  new_event             = Event.new
  new_event.number      = self
  new_event.city        = event.city
  new_event.state       = event.state
  new_event.postalcode  = event.postal_code if event.postal_code
  new_event.country     = event.country
  new_event.status      = event.name
  new_event.status_code = event.type
  new_event.occured_at  = event.occurred_at
  new_event.save
end