如何幹這個代碼並清理我的模型？

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 

正如你所看到的，很多代碼是重複的。當我開始添加十幾個其他運營商（聯邦快遞，美國郵政，DHL等）時，問題就變成了......我的Number模型變得越來越大。

track和update_number之間的唯一真正的區別在於update_number爲，如果周圍的事件比較，以檢查是否從運營商的事件比我已經存儲在數據庫中的最新事件最近使用if last_event and (event.occurred_at > last_event)線。

所以，我怎麼能清理這段代碼，所以我的模式沒有得到這麼胖？

Answer 1

講講你的代碼沒有意義對我來說，該組織是一個有些奇怪，我沒有對你的應用程序看起來像什麼足夠的上下文。 OOP-ish在rails中完成這樣的事情的方式非常簡單;你也可以推出你自己的策略，適配器，甚至是一個Builder模式來做到這一點。

但是，有一些低掛水果，你可以重構你的代碼，以便共用部位是那麼突兀 - 這是一個好一點，但create_events還是很case「Y：

Answer 2

一對夫婦的事情，我會建議：

1. 看那Strategy Pattern，即使Ruby沒有接口，但它會給你如何更好地設計這個功能的一些想法（谷歌Ruby尋找替代品的策略模式）。基本上，你希望有單獨的類來處理你的switch case語句，並在運行時調用合適的switch語句。
2. 嘗試將跟蹤器處理代碼與事件處理代碼分開。例如，我會考慮將每個跟蹤器事件的事件創建/保存邏輯移動到一個單獨的類（或者甚至是跟蹤器實體，如果有的話）。

希望這會有所幫助。

Answer 3

這應該使用戰略模式解決。對於每個可能的跟蹤器（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 -instance的）決定，並應一次獲取並存儲一個實例變量中。

此外，我想補充的名稱應該是有意義的，這樣一類Number不健全的意義對我來說夠。名稱應該表達意圖，並且最好匹配問題域中的名稱和概念。