2016-08-29 4 views
0

私のコードでメモを使って、何度も何度も関数を呼び出さないように言われました。私の実装はそれを使うのに最も良い方法ですか?それは冗長なようだ。どのように私は初期化機能を取り除くことができます助言してください。私のコードでmemoizationを使う

class OrderService 
    def initialize 
    @current_orders = current_orders 
    end 

    def orders_acceptance 
    @current_orders. 
     with_statuses(:acceptance). 
     select do |order| 
     order.acceptance? if order.shopper_notified_at? 
     end 
    end 

    def orders_start 
    @current_orders. 
     with_statuses(:start). 
     select do |order| 
     order.start? 
     end 
    end 


    private 

    def current_orders 
    @current_orders ||= begin 
     Order.includes(:timestamps). 
     with_statuses(
      [ 
      :acceptance, 
      :start 
      ] 
     ) 
    end 
    end 
end 
+1

正直なところ私はこの特定のケースが多くの改善を提供しているとは思わないが、あなたのメソッドはアクティブなレコード関係を返すが、これは私が間違っているとは思わないが、それはこの部分でとても役に立つと思います。 –

+0

'with_statuses'とは何ですか? –

+0

ここを見てください:https://codereview.stackexchange.com/ – Felix

答えて

1

2ヒント:

  1. はコンストラクタで直接current_ordersを呼び出さないでください。 orders_startまたはorders_acceptanceのいずれかを呼び出しているときは、注文を初めて読み込む必要があります。リクエスト処理が開始されたときに早くこのサービスを初期化するリスクはありますが、ビジネスルールによってはこれらのメソッドのどちらも実行されません。その場合、あなたはdbを呼び出しましたが、決してその結果を消費しませんでした。

  2. orders_acceptanceorders_startの両方で、@current_ordersインスタンス変数を使用しています。大丈夫ですが、current_ordersメソッドを複数回呼び出すだけで、結果は同じです。

0

イニシャライザでは何もする必要はありません。

最初にcurrent_ordersが呼び出されると、値がメモされます。これは「標準」メモです。

また、Mohammad氏が指摘しているように、あなたのメソッドはAR関係を返しています。メソッドは次のようにする必要があります。

@current_orders ||= Order.includes(:timestamps) 
         .with_statuses([:acceptance,:start]) 
         .to_a 
end 

また、なぜこのメモを取りたいですか? current_ordersは最新のものであり、古いものではありません。

関連する問題