开发者

Refactor ruby helper method

开发者 https://www.devze.com 2023-01-19 12:31 出处:网络
I have a helper method which checks whether the collection of objects is empty? If not then it checks each one to make sure that the the existing event_id is not the @current_event.id.

I have a helper method which checks whether the collection of objects is empty? If not then it checks each one to make sure that the the existing event_id is not the @current_event.id.

Here is my crack at it:

开发者_JAVA百科
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

Update: This helper method sets the form to build new children objects if the conditions pass. I've updated the example above. btw, it doesn't need to be a single line. I just wanted something cleaner than what I have above.


Without knowing what you're actually doing inside the blocks, it's difficult to give the best solution.

For the most part, all you could really do is select before executing the logic on the filtered collection, rather than testing the logic in the block.

e.g.

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

IMHO it's a little bit neater, and perhaps more rubyish..


Well, I don't know why would you want to put conditions into a single line, but the else block could be rewritten into this:

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


I think you current method is responsible for too many things, my idea is to create a clase with a single responsibility of building answers. That would make your code more readable and also easy to test. A posible implementation would look something like :

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
0

精彩评论

暂无评论...
验证码 换一张
取 消