Это хороший подход к СУХОЙ?

Question

Действие моего контроллера выглядит следующим образом:

# check if user is already working
if current_user.working?
  flash[:error] = I18n.translate 'error.working'    
  redirect_to labor_url and return          
end

# check if user is doing a quest
if current_user.has_tavern_quest?
  flash[:error] = I18n.translate 'error.has_quest'      
  redirect_to labor_url and return          
end

Теперь этот код повторяется в других контроллерах и их действиях.Поэтому я подумал, что было бы неплохо немного СУШИТЬ вещи.Я думал о создании пользовательского метода, например:

def is?(states)
    possible_states = :working, :doing_tavern_quest
    # check states
    # set flash messages, do the same things as above without the redirects
end

Идея состоит в том, что теперь я бы просто использовал что-то подобное в своих действиях:

redirect_to labor_url if current_user.is?(:working, :doing_tavern_quest)

Как вы думаете, это хорошо?идея?Это хороший способ высушить вещи или я мог бы сделать это лучше?

Answer 1

В принципе, с current_user.is?(state) нет ничего плохого, кроме, может быть, имени (is_one_of? ...)

def is?(states)
    possible_states = [:working, :doing_tavern_quest] # note square brackets here
    # check states
    # set flash messages, do the same things as above without the redirects
end

Вы можете извлечь метод из строки перенаправления и создать метод в классе ApplicationController, чтобы код был еще более сухим (если перенаправление находится в том же месте).

Answer 2

Мне нравится шаблон и return .

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

Если контроллер не должен знать подробности, как о:

redirect_to labor_url if current_user.busy? # no parameters

Кстати, методы контроллера ничего не возвращают, вы могли бы сделать:

return redirect_to labor_url if current_user.busy?