2016-08-29 4 views
0

Было сказано использовать memoization в моем коде, чтобы не повторять функцию снова и снова. Является ли моя реализация лучшим способом ее использования? Это кажется излишним. Пожалуйста, сообщите, как я могу избавиться от функции инициализации.использование 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_acceptance и orders_start используется переменная экземпляра @current_orders. Это нормально, но это прекрасно, если вы просто вызываете метод current_orders несколько раз - результат тот же.

0

Вам ничего не нужно делать в инициализаторе.

В первый раз вызывается current_orders, значение будет сохранено в памяти, что является «стандартной» записью.

Кроме того, как указал Мохаммад, ваш метод возвращает отношение AR. Метод должен быть:

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

Также, почему вы хотите запомнить это? current_orders должен быть текущим, не устарелым.

Смежные вопросы