2010-03-13 11 views
1

Comment sécher le code ci-dessous? Dois-je configurer un tas d'ELSE? Je trouve habituellement le "si cela est rencontré, arrêtez", "si cela est rencontré, arrêtez", plutôt qu'un tas d'ifs imbriqués.Ruby Actions: Comment éviter un tas de retours pour arrêter l'exécution?

J'ai découvert que redirect_to et ne rendent pas arrêter l'exécution de l'action ...

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 

Répondre

0

Ajouter and return false à la fin d'un redirect_to ou d'arrêter l'exécution render à ce moment-là. Cela devrait aider à nettoyer les choses pour vous.

+0

renvoyer false provoque Rails à rendre nothign? – Alexandre

3

Vous pouvez procéder comme suit pour réduire le code.

1) Utilisation

return redirect_to(..) 

au lieu de

redirect_to(..) 
return 

2) Extraire le code flash et redirect_to un procédé commun. Créez une nouvelle méthode pour rediriger vers une URL donnée après l'affichage d'un message flash.

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 

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

Remarque J'ai tronqué le code ci-dessus dans certains endroits pour une meilleure lisibilité.

+0

Salut Kandada. Vous pensez que le code peut être refactorisé pour éviter de revenir tôt? – Alexandre

+0

La combinaison de 'return' et' redirect_to' réduit déjà la redondance. Pouvez-vous donner plus de détails dans votre question? –

0

Vous pouvez également factoriser les étapes en méthodes séparées. Ainsi, le code de fin ressemblerait à quelque chose comme:

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

Pour une affacturage ressemblant à:

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 

Ceci est fondamentalement juste en utilisant vrai/faux avec & & « s pour faire le début de la sortie plutôt que d'utiliser retour, mais il semble me lire plus facile, à moi. Vous devrez encore appeler render dans les autres méthodes, mais cela ne devrait pas être trop gros.

La distinction entre l'ordre de confirmation et le dernier achat semble également étrange, mais c'est peut-être un artefact de la façon dont fonctionne le confirm_payment.