1

以下のコードをどのように DRY できますか? たくさんの ELSE をセットアップする必要がありますか? 私は通常、ネストされたifの束ではなく、「これが満たされた場合は停止する」、「これが満たされた場合は停止する」を見つけます。

redirect_to と render がアクションの実行を停止しないことを発見しました...

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
4

3 に答える 3

3

コードを減らすために、次のことを行うことができます。

1）使用

return redirect_to(..)

それ以外の

redirect_to(..)
return

flash2）とredirect_toコードを共通のメソッドに抽出します。

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

フラッシュメッセージを表示した後、特定のURLにリダイレクトする新しいメソッドを作成します。

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

読みやすくするために、上記のコードをいくつかの場所で切り捨てていることに注意してください。

于 2010-03-13T23:59:03.130 に答える
0

ステップを別の方法に分解することもできます。したがって、終了コードは次のようになります。

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

次のようなファクタリングの場合:

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

これは基本的に、リターンを使用するのではなく、&& で true/false を使用して早期終了を行うだけですが、私には読みやすいようです。他のメソッドを呼び出すrender必要がありますが、それほど大したことではありません。

確認注文と最後の購入の違いも奇妙に思えますが、おそらくこれは、confirm_payment の動作のアーティファクトです。

于 2010-04-26T08:26:35.877 に答える
0

and return falseその時点で実行を停止するには、redirect_to または render の末尾に追加します。それはあなたのために物事をきれいにするのに役立つはずです.

于 2010-03-13T22:51:24.453 に答える