2010-10-12 65 views
2

我有一個幫助器方法,檢查對象的集合是否爲空?如果不是,那麼它會檢查每一個以確保現有的event_id不是@ current_event.id。重構紅寶石輔助方法

這裏是我的裂紋吧:

def build_answers(question) 
    if question.answers.empty? 
    return question.answers.build 
    else 
    question.answers.each do |a| 
     if a.event_id != @current_event.id 
     return question.answers.build 
     end 
    end 
    end 
end 

更新:這個helper方法設置窗體,以建立新的兒童如果條件傳遞對象。我已經更新了上面的例子。順便說一句,它不需要是一條線。我只想要比上面更清潔的東西。

+1

你能否解釋一下,什麼都打上「邏輯」塊執行的操作? – Jeriko 2010-10-12 14:00:52

+0

您確定此代碼是正確的嗎? 'question.answers.build'不應該在每個''塊內,如果集合是空的,那麼沒有必要調用'question.answers.build'。 – Jeriko 2010-10-12 14:16:53

+0

實際上,它應該是這樣的。我不想放下所有其他代碼,因爲表單非常複雜,但它實際上按預期工作。感謝您的回答。 – 2010-10-12 14:21:15

回答

1

不知道你在塊內實際做什麼,很難提供最好的解決方案。

在大多數情況下,你可以真正做的是select執行對過濾收集邏輯,而不是塊測試邏輯之前。

例如

uncurrent_answers = questions.answers.select{|a| a.event_id != @current_event.id} 
uncurrent_answers.each do |a| 
    #do whatever 
end 

恕我直言,這是一個有點整潔,也許更rubyish ..

+0

選擇塊是我正在尋找(我不知道它叫什麼)。地圖和其他方法只是沒有工作(顯然)。謝謝! – 2010-10-12 14:18:50

+0

@Nate爲了將來的參考,您可能想查看[Enumerable](http://ruby-doc.org/core/classes/Enumerable.html)中的所有其他方法。 – mikej 2010-10-12 15:22:38

+0

感謝您的鏈接。 – 2010-10-14 11:51:54

1

好了,我不知道爲什麼你會希望把條件變成單行線,但其他模塊都可以改寫這個:

question.answers.select {|answer| answer.event_id != @current_event.id }.each 
    {|ans| #.. logic with answer here } 
+0

我給你一個提出選擇選項的點。正是我需要的。謝謝! – 2010-10-12 14:19:33

0

我覺得你目前的方法是負責的東西太多了,我的想法是創建一個化酶與建築答案的單一職責。這會讓你的代碼更具可讀性,並且易於測試。一個更多鈔票實施看起來是這樣的:

def build_answers(question) 
    AnswerBuilder.build(question.answers, @current_event) 
end 

class AnswerBuilder 
    def initialize(answers, current_event) 
    @answers = answers 
    @current_event = current_event 
    end 

    def self.build(answers, current_event) 
    new(answers, current_event).build 
    end 

    def build 
    if answers.empty? 
     answers.build 
    else 
     create_allowed_answers 
    else 
    end 

    private 
    attr_reader :answers, :current_event 

    def create_allowed_answers 
    answers.each do |a| 
     if a.event_id != current_event.id 
     return answers.build 
     end 
    end 
    end 
end