Updated Collection#pop and Collection#shift to pass arguments to Array

* On Ruby 1.8.7+ this will allow you to pop and shift more than 1
  entry off the collection

[#1032 state:resolved]

lib/dm-core/collection.rb

@@ -534,9 +534,10 @@ def insert(offset, *resources)
     #   the last Resource in the Collection
     #
     # @api public
-    def pop
-      return nil unless resource = super
-      resource_removed(resource)
+    def pop(*)
+      if removed = super
+        resources_removed(removed)
+      end
     end
 
     # Removes and returns the first Resource in the Collection
@@ -545,9 +546,10 @@ def pop
     #   the first Resource in the Collection
     #
     # @api public
-    def shift
-      return nil unless resource = super
-      resource_removed(resource)
+    def shift(*)
+      if removed = super
+        resources_removed(removed)
+      end
     end
 
     # Remove Resource from the Collection
@@ -566,8 +568,9 @@ def shift
     #
     # @api public
     def delete(resource)
-      return nil unless resource = super
-      resource_removed(resource)
+      if resource = super
+        resource_removed(resource)
+      end
     end
 
     # Remove Resource from the Collection by offset
@@ -586,8 +589,9 @@ def delete(resource)
     #
     # @api public
     def delete_at(offset)
-      return nil unless resource = super
-      resource_removed(resource)
+      if resource = super
+        resource_removed(resource)
+      end
     end
 
     # Deletes every Resource for which block evaluates to true.

spec/public/shared/collection_shared_spec.rb

@@ -618,24 +618,52 @@
   describe '#pop' do
     before :all do
       @new = @articles.create(:title => 'Sample Article')  # TODO: freeze @new
-
-      @return = @resource = @articles.pop
     end
 
-    it 'should return a Resource' do
-      @return.should be_kind_of(DataMapper::Resource)
-    end
+    describe 'with no arguments' do
+      before :all do
+        @return = @articles.pop
+      end
 
-    it 'should be the last Resource in the Collection' do
-      @resource.should == @new
-    end
+      it 'should return a Resource' do
+        @return.should be_kind_of(DataMapper::Resource)
+      end
+
+      it 'should be the last Resource in the Collection' do
+        @return.should == @new
+      end
 
-    it 'should remove the Resource from the Collection' do
-      @articles.should_not be_include(@resource)
+      it 'should remove the Resource from the Collection' do
+        @articles.should_not be_include(@new)
+      end
+
+      it 'should orphan the Resource' do
+        @return.collection.should_not equal(@articles)
+      end
     end
 
-    it 'should orphan the Resource' do
-      @resource.collection.should_not equal(@articles)
+    if RUBY_VERSION >= '1.8.7'
+      describe 'with a limit specified' do
+        before :all do
+          @return = @articles.pop(1)
+        end
+
+        it 'should return an Array' do
+          @return.should be_kind_of(Array)
+        end
+
+        it 'should return the expected Resources' do
+          @return.should == [ @new ]
+        end
+
+        it 'should remove the Resource from the Collection' do
+          @articles.should_not be_include(@new)
+        end
+
+        it 'should orphan the Resource' do
+          @return.each { |resource| resource.collection.should_not equal(@articles) }
+        end
+      end
     end
   end
 
@@ -941,26 +969,51 @@
   it { @articles.should respond_to(:shift) }
 
   describe '#shift' do
-    before :all do
-      @new = @articles.create(:title => 'Sample Article')
+    describe 'with no arguments' do
+      before :all do
+        @return = @articles.shift
+      end
 
-      @return = @resource = @articles.shift
-    end
+      it 'should return a Resource' do
+        @return.should be_kind_of(DataMapper::Resource)
+      end
 
-    it 'should return a Resource' do
-      @return.should be_kind_of(DataMapper::Resource)
-    end
+      it 'should be the first Resource in the Collection' do
+        @return.key.should == @article.key
+      end
 
-    it 'should be the first Resource in the Collection' do
-      @resource.key.should == @article.key
-    end
+      it 'should remove the Resource from the Collection' do
+        @articles.should_not be_include(@return)
+      end
 
-    it 'should remove the Resource from the Collection' do
-      @articles.should_not be_include(@resource)
+      it 'should orphan the Resource' do
+        @return.collection.should_not equal(@articles)
+      end
     end
 
-    it 'should orphan the Resource' do
-      @resource.collection.should_not equal(@articles)
+    if RUBY_VERSION >= '1.8.7'
+      describe 'with a limit specified' do
+        before :all do
+          @return = @articles.shift(1)
+        end
+
+        it 'should return an Array' do
+          @return.should be_kind_of(Array)
+        end
+
+        it 'should return the expected Resources' do
+          @return.size.should == 1
+          @return.first.key.should == @article.key
+        end
+
+        it 'should remove the Resource from the Collection' do
+          @articles.should_not be_include(@article)
+        end
+
+        it 'should orphan the Resource' do
+          @return.each { |resource| resource.collection.should_not equal(@articles) }
+        end
+      end
     end
   end
 

spec/public/shared/finder_shared_spec.rb

@@ -846,7 +846,7 @@
       end
     end
 
-    describe 'with limit specified' do
+    describe 'with a limit specified' do
       before :all do
         @return = @resources = @articles.first(1)
       end
@@ -882,7 +882,7 @@
       end
     end
 
-    # describe 'with limit specified', 'after prepending to the collection' do
+    # describe 'with a limit specified', 'after prepending to the collection' do
     #   before :all do
     #     @return = @resources = @articles.unshift(@other).first(1)
     #   end
@@ -904,7 +904,7 @@
     #   end
     # end
 
-    describe 'with limit and query specified' do
+    describe 'with a limit and query specified' do
       before :all do
         @return = @resources = @articles.first(1, :content => 'Sample')
       end
@@ -1222,7 +1222,7 @@
       # end
     end
 
-    describe 'with limit specified' do
+    describe 'with a limit specified' do
       before :all do
         @return = @resources = @articles.last(1)
       end
@@ -1258,7 +1258,7 @@
       end
     end
 
-    # describe 'with limit specified', 'after appending to the collection' do
+    # describe 'with a limit specified', 'after appending to the collection' do
     #   before :all do
     #     @return = @resources = @articles.push(@other).last(1)
     #   end
@@ -1280,7 +1280,7 @@
     #   end
     # end
 
-    describe 'with limit and query specified' do
+    describe 'with a limit and query specified' do
       before :all do
         @return = @resources = @articles.last(1, :content => 'Sample')
       end