current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite
1

I occasionally find myself bumping into code like this (either in other projects or banging out initial prototypes myself):

 if @product.save 
   if current_member.role == "admin" 
     redirect_to krowd_path(@product) 
   else 
     redirect_to new_product_offer_path(@product) 
   end 
 else 
   render :new 
 end

What is a good way to avoid this type of situation all together?

share|improve this question
    
Use small functions that call each other and do not be afraid of multiple return points within one function. –  Leonid Apr 22 '12 at 18:50
2  
I am not familiar with rails, but you can avoid one level of nesting by checking the exceptional/rare condition first and returning. –  rahul Apr 22 '12 at 19:05
1  
codinghorror.com/blog/2006/01/flattening-arrow-code.html –  Leonid Apr 22 '12 at 19:23

2 Answers 2

up vote 3 down vote accepted

One suggestion is to make the roles two classes, initialize them based on the role and call the save function.

 class UserRole
    def save(product)
     redirect_to new_product_offer_path(product) 
    end
 end
 class AdminRole < UserRole
    def save(product)
     redirect_to krowd_path(product) 
    end
 end
 def create_role(r)
    case r
    when :admin
        return AdminRole.new()
    else
        return UserRole.new()
    end
 end


 ....
 role = create_role(current_member.role)

 if @product.save 
   role.save(@product) 
 else 
   render :new 
 end
share|improve this answer
up vote 0 down vote

If it matches to your requirements you could implement something like:

redirect_to_first_accessible(
  krowd_path(@product), 
  new_product_offer_path(@product))

If you use something like CanCan gem, gererally it's defined which action is accessible to certain roles.

share|improve this answer
    
I considered cancan for this case, but there are instances where it's not access based. The arrow code articlesmsup those generic cases, but cancan definitely helps separate when it comes to access. –  cmhobbs Apr 30 '12 at 1:49

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.