Refactor ruby ​​helper метод

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

Ну, я не знаю, почему вы хотите поместить условия в одну строку, но блок else можно переписать так:

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

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

Я думаю, что ваш текущий метод ответственен за слишком много вещей, моя идея состоит в том, чтобы создать предложение с единственной ответственностью - строить ответыЭто сделает ваш код более читабельным, а также легко тестируемым.Возможная реализация будет выглядеть примерно так:

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