Refactor ruby helper method
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
精彩评论