Is this a good approach to DRY?

My controller action is like :

# 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 d开发者_如何学编程oing a quest
if current_user.has_tavern_quest?
  flash[:error] = I18n.translate 'error.has_quest'      
  redirect_to labor_url and return          
end

Now, this is some code that seems to be repeating itself in other controllers and their actions as well. Therefore, i thought that it would be a good idea to DRY things a little. I thought of creating a user method like :

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

The idea is that i would now just use something like that in my actions :

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

Do you think that is a good idea ? Is it a good way to DRY things up or i could do it in a better manner ?

---

I like the and return pattern.

But unless there are lots of different dynamic conditions, why not encapsulate the User state within the model as much as possible?

Unless the controller needs to know details, how about:

redirect_to labor_url if current_user.busy? # no parameters

BTW, controller methods don't return anything, you could possibly do:

return redirect_to labor_url if current_user.busy?

---

Nothing wrong in principle with current_user.is?(state) except maybe the name (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

You could extract method the redirect line and create a method in the ApplicationController class so that the code is even drier (if the redirection is to the same place.)