重构 ruby​​ 辅助方法

Question

我有一个辅助方法来检查对象集合是否为空？如果不是，则检查每一个以确保现有 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

更新：如果条件满足，此辅助方法将表单设置为构建新的子对象。我已经更新了上面的示例。顺便说一句，它不需要是单行。我只是想要一些比我上面的更干净的东西。

Answer 1

我认为您当前的方法负责太多事情，我的想法是创建一个仅负责构建答案的类。这将使您的代码更具可读性并且易于测试。一个可能的实现看起来像：

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

Answer 2

如果不知道您在块内实际在做什么，很难给出最佳解决方案。

在大多数情况下，您真正​​可以做的就是在对过滤后的集合执行逻辑之前select，而不是测试块中的逻辑。

例如

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

恕我直言，它有点整洁，也许更红宝石..

Answer 3

好吧，我不知道为什么要将条件放在一行中，但是 else 块可以重写为：

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