Лучший способ высушить это и уточнить код лучше

Question

Я ищу лучший способ сохранить мои контроллеры чистыми и читаемыми. Посмотрите на это действие контроллера:

  def start
    @work_hours = params[:work_hours].to_i

    redirect_to labor_url, :flash => {:error => I18n.t('error.invalid_post')} and return unless (1..8).include? @work_hours
    redirect_to labor_url, :flash => {:error => I18n.t('error.working')} and return if current_user.working?
    redirect_to labor_url, :flash => {:error => I18n.t('error.has_quest')} and return if current_user.has_tavern_quest?

    redirect_to labor_path 
  end

Как вы можете видеть, они делают то же самое, если возникает условие. Они устанавливают флеш-сообщение и перенаправляют на URL (и возвращают). Хотя мне кажется, что с точки зрения разъяснения это нормально, я не могу не заметить небольшое повторение в перенаправлениях, и мне не нравится устанавливать flash [: error] с переводом в такой ужасной манере.

Как вы думаете, это можно сделать лучше, СУХОЙ и более читаемой?

Answer 1

URL-адрес одинаков для всех перенаправлений (если я вижу правильно, нет разницы между URL-адресом и путем), поэтому я бы изменил код следующим образом:

def start
  @work_hours = params[:work_hours].to_i

  flash[:error] = I18n.t('error.invalid_post') unless (1..8).include? @work_hours
  flash[:error] = I18n.t('error.working') if current_user.working?
  flash[:error] = I18n.t('error.has_quest') if current_user.has_tavern_quest?

  redirect_to labor_path 
end

Итак: при необходимости установите флэш-память,и перенаправить на labor_path во всех случаях.Помогает ли это?

Если в случае ошибки вам нужно будет перенаправить на что-то другое, сделайте что-то вроде:

def start
  @work_hours = params[:work_hours].to_i

  flash[:error] = I18n.t('error.invalid_post') unless (1..8).include? @work_hours
  flash[:error] = I18n.t('error.working') if current_user.working?
  flash[:error] = I18n.t('error.has_quest') if current_user.has_tavern_quest?

  redirect_to labor_error_path and return if flash[:error]
  redirect_to labor_path 
end

Если условия не являются взаимоисключающими, я бы написал это так:

def start
  @work_hours = params[:work_hours].to_i

  flash[:error] = unless (1..8).include? @work_hours 
    I18n.t('error.invalid_post') 
  elsif current_user.working?
    I18n.t('error.working')
  elsif current_user.has_tavern_quest?
    I18n.t('error.has_quest') 
  else
    nil
  end

  redirect_to labor_error_path and return if flash[:error]
  redirect_to labor_path 
end

Я не совсем уверен, явно ли нужен else nil.Это помогает?