Ruby Actions: How to avoid a bunch of returns to halt execution?

Question

How can I DRY the code below? Do I have to setup a bunch of ELSEs ? I usually find the "if this is met, stop", "if this is met, stop", rather than a bunch of nested ifs.

I discovered that redirect_to and render don't stop the action execution...

def payment_confirmed    
  confirm_payment do |confirmation|    
    @purchase = Purchase.find(confirmation.order_id)
    unless @purchase.products_match_order_products?(confirmation.products)
      # TODO notify the buyer of problems
      return
    end

    if confirmation.status == :completed
      @purchase.paid!                     
      # TODO notify the user of completed purchase
      redirect_to purchase_path(@purchase)
    else      
      # TODO notify the user somehow that thigns are pending
    end   

    return
  end

  unless session[:last_purchase_id]
    flash[:notice] = 'Unable to identify purchase from session data.'
    redirect_to user_path(current_user) 
    return
  end

  @purchase = Purchase.find(session[:last_purchase_id]) 

  if @purchase.paid?
    redirect_to purchase_path(@purchase)       
    return
  end

  # going to show message about pending payment
end

Answer 1

Add and return false to the end of a redirect_to or render to halt execution at that point. That should help clean things up for you.

Answer 2

You can do the following to reduce the code.

1) Use

return redirect_to(..)

instead of

redirect_to(..)
return

2) Extract the flash and redirect_to code to a common method.

def payment_confirmed    
  confirm_payment do |confirmation|    
    @purchase = Purchase.find(confirmation.order_id)        
    return redirect_with_flash(...) unless @purchase.products_match_..(..)

    return redirect_with_flash(...) unless confirmation.status == :completed

    @purchase.paid! 
    return redirect_to(...)
  end

  return redirect_with_flash(...) unless session[:last_purchase_id]      

  @purchase = Purchase.find(session[:last_purchase_id]) 
  return redirect_to(...) if @purchase.paid?

  # going to show message about pending payment
end

Create a new method to redirect to a given url after showing a flash message.

def redirect_with_flash url, message
  flash[:notice] = message
  redirect_to(url)
end

Note I have truncated the code above in some places for readability.

Answer 3

You could also factor out the steps into seperate methods. So the ending code would look something like:

def payment_confirmed    
  confirm_payment do |cnf|    
    confirmation_is_sane?(cnf) && purchase_done?(cnf)
    return
  end
  has_last_purchase? && last_purchase_paid?
end

For a factoring looking like:

def confirmation_is_sane?(confirmation)
   @purchase = Purchase.find(confirmation.order_id)
   unless @purchase.products_match_order_products?(confirmation.products)
     # TODO notify the buyer of problems and render
      return false 
    end
   true
end 
def purchase_done?(confirmation)
   if confirmation.status == :completed
      @purchase.paid!                     
      # TODO notify the user of completed purchase
      redirect_to purchase_path(@purchase)
      return false
    else      
      # TODO notify the user somehow that thigns are pending and render
      return true
    end   
end
def has_last_purchase?
  unless session[:last_purchase_id]
    flash[:notice] = 'Unable to identify purchase from session data.'
    redirect_to user_path(current_user) 
    return false
  end

  @purchase = Purchase.find(session[:last_purchase_id]) 
  return true
end
def last_purchase_paid?
  if @purchase.paid?
    redirect_to purchase_path(@purchase)       
    return false
  end
  # going to show message about pending payment
  return true
end

This is basically just using true/falses with &&'s to do the early exiting rather than using return, but it seems to read easier, to me. You'd still have to call render in the other methods, but that shouldn't be too big of a deal.

The distinction between the confirmation order and the last purchase seems strange as well, but perhaps this is an artifact of the way confirm_payment works.