[ ruby-Patches-1900 ] Pointless argc check in Array#select

Patches item #1900, was opened at 2005-05-12 09:33
You can respond by visiting: 
http://rubyforge.org/tracker/?func=detail&atid=1700&aid=1900&group_id=426

Category: Ruby1.8
Group: None
Status: Open
Resolution: None
Priority: 3
Submitted By: Daniel Berger (djberg96)
Assigned to: Nobody (None)
Summary: Pointless argc check in Array#select

Initial Comment:
Ruby 1.8.3 p1

Pointless argc check in Array#select:

--- array.orig  Thu May 12 08:27:15 2005
+++ array.c     Thu May 12 08:32:44 2005
@@ -1807,14 +1807,12 @@
     VALUE result;
     long i;
 
-    if (argc > 0) {
-       rb_raise(rb_eArgError, "wrong number of arguments (%d for 0)", argc);
-    }
     result = rb_ary_new2(RARRAY(ary)->len);
+
     for (i = 0; i < RARRAY(ary)->len; i++) {
-       if (RTEST(rb_yield(RARRAY(ary)->ptr[i]))) {
-           rb_ary_push(result, rb_ary_elt(ary, i));
-       }
+       if (RTEST(rb_yield(RARRAY(ary)->ptr[i]))) {
+               rb_ary_push(result, rb_ary_elt(ary, i));
+       }
     }
     return result;
 }
@@ -3017,7 +3015,7 @@
     rb_define_method(rb_cArray, "collect!", rb_ary_collect_bang, 0);
     rb_define_method(rb_cArray, "map", rb_ary_collect, 0);
     rb_define_method(rb_cArray, "map!", rb_ary_collect_bang, 0);
-    rb_define_method(rb_cArray, "select", rb_ary_select, -1);
+    rb_define_method(rb_cArray, "select", rb_ary_select, 0);
     rb_define_method(rb_cArray, "values_at", rb_ary_values_at, -1);
     rb_define_method(rb_cArray, "delete", rb_ary_delete, 1);
     rb_define_method(rb_cArray, "delete_at", rb_ary_delete_at_m, 1);


--- test_array.orig     Thu May 12 08:33:32 2005
+++ test_array.rb       Thu May 12 10:01:58 2005
@@ -1,6 +1,9 @@
 require 'test/unit'
 
 class TestArray < Test::Unit::TestCase
+   def setup
+      @a = []
+   end
   def test_array
     assert_equal([1, 2, 3, 4], [1, 2] + [3, 4])
     assert_equal([1, 2, 1, 2], [1, 2] * 2)
@@ -98,4 +101,24 @@
     x.concat(x)
     assert_equal([1,2,3,1,2,3], x)
   end
+
+   def test_find_all
+      assert_respond_to(@a, :find_all)
+      assert_respond_to(@a, :select)       # Alias
+      assert_equal([], @a.find_all{ |obj| obj == "foo"})
+
+      # Should this raise a warning or error?  Should a block be mandatory?
+      assert_nothing_raised{ @a.find_all }
+
+      # This one is specifically relevant to the diff
+      assert_raises(ArgumentError){ @a.find_all("foo") }
+
+      @a.push("foo", "bar", "baz", "baz", 1, 2, 3, 3, 4)
+      assert_equal(["baz","baz"], @a.find_all{ |obj| obj == "baz" })
+      assert_equal([3,3], @a.find_all{ |obj| obj == 3 })
+   end
+
+   def teardown
+      @a = nil
+   end
 end

>ruby test_array.rb
Loaded suite test_array
Started
........
Finished in 0.031893 seconds.

8 tests, 46 assertions, 0 failures, 0 errors

Regards,

Dan

PS - I think there's some extra info in the diff because I replaced tabs with spaces (a minor annoyance with the source code itself, btw).

----------------------------------------------------------------------

You can respond by visiting: 
http://rubyforge.org/tracker/?func=detail&atid=1700&aid=1900&group_id=426