From f6a7b17a8acfa7633762d665ab218a6478d32a98 Mon Sep 17 00:00:00 2001 From: Clemens Kofler Date: Wed, 12 Apr 2017 03:32:09 +0200 Subject: [PATCH] Optimization: Optimize CollectionDecorator Enumerable behavior (#707) * Optimization: Prevent duplicate iteration * Don't delegate Enumerable instance methods since CollectionDecorator includes Enumerable itself --- lib/draper/collection_decorator.rb | 19 +++++++++++++- spec/draper/collection_decorator_spec.rb | 33 ++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/lib/draper/collection_decorator.rb b/lib/draper/collection_decorator.rb index b3e4cee..105fab2 100644 --- a/lib/draper/collection_decorator.rb +++ b/lib/draper/collection_decorator.rb @@ -15,7 +15,7 @@ module Draper # to each item's decorator. attr_accessor :context - array_methods = Array.instance_methods - Object.instance_methods + array_methods = Array.instance_methods - Object.instance_methods - Enumerable.instance_methods delegate :==, :as_json, *array_methods, to: :decorated_collection # @param [Enumerable] object @@ -42,6 +42,23 @@ module Draper @decorated_collection ||= object.map{|item| decorate_item(item)} end + # Optimization to prevent unnecessary iteration (useful for larger collections). + # Iterates over the collection, decorating objects as it goes. If the decorated collection is + # already set, iterate over it instead. + def each(&block) + if @decorated_collection + @decorated_collection.each(&block) + else + @decorated_collection = [] + object.each do |item| + decorated_item = decorate_item(item) + @decorated_collection << decorated_item + + block.call(decorated_item) + end + end + end + delegate :find, to: :decorated_collection def to_s diff --git a/spec/draper/collection_decorator_spec.rb b/spec/draper/collection_decorator_spec.rb index b704fe1..cb67b97 100644 --- a/spec/draper/collection_decorator_spec.rb +++ b/spec/draper/collection_decorator_spec.rb @@ -288,5 +288,38 @@ module Draper end end + describe "#each" do + it "iterates over the collection, decorating as it goes" do + collection = [Product.new] + decorator = CollectionDecorator.new(collection) + + expect(collection).to_not receive(:map) + decorator.each { |product| product.decorated? } + expect(decorator.instance_variable_get(:@decorated_collection)[0]).to be_decorated + end + + it "uses decorated_collection if already set" do + decorated_collection = double(:decorated_collection) + decorator = CollectionDecorator.new([]) + decorator.instance_variable_set(:@decorated_collection, decorated_collection) + + expect(decorated_collection).to receive(:each) + + decorator.each + end + end + + describe "Enumerable methods" do + it "doesn't delegate Enumerable methods to its decorated collection" do + decorated_collection = double(:decorated_collection) + decorator = CollectionDecorator.new([]) + decorator.instance_variable_set(:@decorated_collection, decorated_collection) + + expect(decorated_collection).to_not receive(:map) + + decorator.map + end + end + end end