我怎样才能干燥这段代码并清理我的模型？

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 比较，以检查来自运营商的事件是否更新比我使用 if last_event and (event.occurred_at > last_event) 行存储在数据库中的最新事件。

那么我怎样才能清理这段代码，这样我的模型就不会变得那么胖呢？

Answer 1

我建议的几件事：

1. 查看策略模式，尽管 Ruby 没有接口，但它会给你一些关于如何更好地设计这个功能的想法（谷歌搜索 Ruby 的策略模式来找到替代方案）。基本上，您希望有单独的类来处理 switch case 语句并在运行时调用适当的类。
2. 尝试将跟踪器处理代码与事件处理代码分开。例如，我会考虑将每个跟踪器事件的事件创建/保存逻辑移动到一个单独的类（甚至移动到跟踪器实体，如果有的话）。

希望这有帮助。

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

您的代码中的某些内容对我来说没有意义，组织有点奇怪，而且我对您的应用程序的外观没有足够的背景信息。在 Rails 中完成类似的 OOP 方式非常简单；您还可以推出自己的策略、适配器，甚至构建器模式来执行此操作。

但是，有一些容易实现的目标，您可以重构代码，使公共部分不那么引人注目——这更好一点，但是 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