2015-11-03 142 views
0

我有一個方法,我正在重構,我知道有重構它的潛力,但我很不確定它應該如何最有效地完成。重構幾個if else語句

# Collection of Users questions 
    def all_questions 
    current_week = Time.zone.now.strftime('%V').to_i 

    # biweekly 
    odd_or_even_week = current_week.odd? ? 'odd_weeks' : 'even_weeks' 

    # monthly 
    beginning_week_of_month = 
     Time.zone.now.beginning_of_month.strftime('%V').to_i 
    end_week_of_month  = 
     Time.zone.now.end_of_month.strftime('%V').to_i 

    # quarter 
    beginning_week_of_quarter = 
     Time.zone.now.beginning_of_quarter.strftime('%V').to_i 
    end_week_of_quarter  = 
     Time.zone.now.end_of_quarter.strftime('%V').to_i 

    # User's current week questions 
    group_questions.weekly + questions.weekly + 
     questions.send(odd_or_even_week.to_sym) + 
     group_questions.send(odd_or_even_week.to_sym) + 
    if current_week == beginning_week_of_month then questions.start_of_month + group_questions.start_of_month  else [] end + 
    if current_week == end_week_of_month   then questions.end_of_month  + group_questions.end_of_month   else [] end + 
    if current_week == beginning_week_of_quarter then questions.start_of_quarter + group_questions.start_of_quarter  else [] end + 
    if current_week == end_week_of_quarter  then questions.end_of_quarter + group_questions.end_of_quarter  else [] end 
    end 

這是我的方法。我實質上在做的是檢查當前周是否匹配已分配給不同變量的幾個標準之一。如果當前周匹配,然後我想要添加一個數組到列表中。

我與重構有一些較小的問題說如果else語句是,如果我沒有一個false作爲一個空數組回退,然後在串聯我會有兩個++旁邊 - 其他因爲它會得到前面的數組,如果在midle中是空的,爲該數組添加+運算符。由此產生一個數組。

問題和group_questions是協會,要求他們是枚舉的方法,看起來像這樣有關問題的模式:

enum frequency: { weekly: 0, odd_weeks: 1, even_weeks: 2, 
        start_of_month: 3, end_of_month: 4, 
        start_of_quarter: 5, end_of_quarter: 6 } 

有誰知道他們會如何重構這個走?

+0

'questions'和'group_questions'是關聯嗎?什麼是他們的方法?作用域? –

+0

問題和group_questions是關聯yes。調用它們的方法是枚舉。我會更新我原來的帖子,使其更清晰 –

+1

我會將所有問題查詢和所有小組問題組合在一起。我還會考慮創建一個對象,它只是解決哪些問題並返回它們。 –

回答

0
def enums_required 
    # do all time calculations here 
    frequency_values = [Conversation.frequency[:weekly]] # always want weekly status 
    frequency_values << Conversation.frequency[odd_or_even_week] 
    frequency_values << Conversation.frequency[:start_of_month] if current_week == beginning_week_of_month 
    frequency_values << Conversation.frequency[:end_of_month] if current_week == end_week_of_month 
    frequency_values << Conversation.frequency[:start_of_quarter] if current_week == beginning_week_of_quarter 
    frequency_values << Conversation.frequency[:end_of_quarter] if current_week == end_week_of_quarter 
    frequency_values 
end 

不要這樣做

odd_or_even_week = current_week.odd? ? 'odd_weeks' : 'even_weeks' 

然後調用.to_sym

只寫一個符號

odd_or_even_week = current_week.odd? ? :odd_weeks : :even_weeks 

現在,在你的方法,你應該能夠做到

freq = enums_required 
group_questions.where(frequency: freq) + questions.where(frequency: freq) 

可能會失敗,因爲我在rails中使用了枚舉。這基本上是一箇中間重構,可以幫助你順利完成任務,但決不會完成最好的任務。