Блог

Главная - Вопросы по программированию - Улучшение кода ruby

Улучшение кода ruby

#ruby-on-rails #ruby

#ruby-on-rails #ruby

Вопрос:

люди говорили мне, что на данный момент у меня неправильный код. Основная идея этого метода заключается в создании 1 события, если пользователь нажал один раз, и ряда событий, если пользователь нажимал ежедневно или еженедельно. Все работает хорошо, но код такой громоздкий.

   def create
@event = Event.new(event_params
@event.start_time = DateTime.parse(params[:start_time], "%Y-%m-%d %H:%i")
@event.end_time = DateTime.parse(params[:end_time], "%Y-%m-%d %H:%i")
@event.user_id = current_user.id
@event.update_attributes(:repeat_id => @event.id) if @event.save
respond_to do |format|
  if @event.save
    format.html { redirect_to persons_profile_path }
  else
    format.html { render :new }
    format.json { render json: @event.errors, status: :unprocessable_entity }
  end
interval = 60 if @event.repeat =='daily'
interval = 20 if @event.repeat =='weekly'
#creating row of events
if @event.repeat != 'once'
  (1..interval).each do |i|
    @event = Event.new(event_params)
    @event.start_time = DateTime.parse(params[:start_time], "%Y-%m-%d %H:%i")
    @event.end_time = DateTime.parse(params[:end_time], "%Y-%m-%d %H:%i")
    @event.user_id = current_user.id
    if @event.repeat =='daily'
      @event.start_time = DateTime.parse(@event.start_time.to_s)   i.day
      @event.end_time = DateTime.parse(@event.end_time.to_s)   i.day
    end
    if @event.repeat =='weekly'
      @event.start_time = DateTime.parse(@event.start_time.to_s)   i.week
      @event.end_time = DateTime.parse(@event.end_time.to_s)   i.week
    end
    @event.update_attributes(:repeat_id => k) if @event.save
    @event.save
  end
end

Я не могу найти другого решения. Есть идеи? Не обращайте внимания на методы синтаксического анализа по дате, это необходимо для JS.

Комментарии:

1. Я не уверен, что вы подразумеваете под рядом событий, также может показаться, что вам не хватает a ) во второй строке. Мой главный вопрос в том, не работает ли код или проблема только в том, что он написан неэффективно? Если это произойдет позже, это следует перенести в code review, если это первое, вам нужно добавить больше деталей.

2. В этом коде много дублирования, что сбивает с толку его смысл. Одна из первых вещей, которые нужно сделать, это попытаться выполнить операцию, подобную DateTime.parse определенной части данных, один раз, а затем повторно использовать это значение. Вы также можете свернуть свой if код в a case , чтобы выяснить, какое смещение использовать, а затем добавить это смещение к обоим значениям.

Ответ №1:

Я думаю, проблема в том, что вы используете контроллеры для выполнения слишком большой работы, и это трудно читать. Обычно контроллеры выглядят так:

 def create
  @client = Client.new(params[:client])
  if @client.save
    redirect_to @client
  else
    render "new"
  end
end

Найдите минутку и подумайте о своих данных. Вы создаете события. Вы хотите, чтобы ваши события отображались подряд, если кнопка создать нажимается ежедневно или еженедельно. Итак, мы думаем о том, как его отобразить, это проблема css / html, а не что-то, связанное с контроллерами.

Причина, по которой люди называют ваш код неправильным, заключается в том, что вы делаете что-то очень нетипичное. Если бы вы отправили этот код в школу или на проект, вы получили бы низкий балл, потому что вы заставляете всех остальных усердно работать, чтобы понять вашу работу, вместо того, чтобы следовать тому же базовому стилю, которому все обучались.

Метки: