2015-11-03 66 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中使用了枚举。这基本上是一个中间重构,可以帮助你顺利完成任务,但决不会完成最好的任务。