Bug #7437: Array#delete(obj) should return obj when there is an object that is equal in the array

• Status: Closed
• Assignee: marcandre (Marc-Andre Lafortune)
• Target version: 2.0.0
• ruby -v: ruby 2.0.0dev (2012-11-25 trunk 37839) [x86_64-darwin12.2.1]

[hasari (Hiro Asari)]

According to http://www.ruby-doc.org/core-1.9.3/Array.html#method-i-delete, Array#delete(obj) should return "obj" when there are objects in the array that are "equal to obj" (internally, "==" is used, it seems).

Notice that the documentation does not state that the return value is an element of the array itself. However, 1.9.3 and trunk both return a member of the Array, rather than the argument.

This issue was raised in https://github.com/jruby/jruby/issues/411

#!/usr/bin/env ruby

class Foo
attr_reader :name, :age

def initialize name, age
@name = name
@age = age
end

def == other
other.name == name
end
end

foo1 = Foo.new "John Shahid", 27
foo2 = Foo.new "John Shahid", 28
array = [foo1]
temp = array.delete foo2 # => foo1, not foo2

[hasari (Hiro Asari)]

• File ruby-7437.patch ruby-7437.patch added

Here's the patch. Where should the tests go? RubySpec?

[marcandre (Marc-Andre Lafortune)]

• Category set to core
• Target version set to 2.0.0

Indeed, the documentation does not match the code and there is no test for this.

It was clearly Matz' intention to return the (last) deleted element: https://bugs.ruby-lang.org/projects/ruby-trunk/repository/revisions/18527

This is the most helpful behavior anyways.

I'll note that the documentation is clearly wrong in the second form too.

Unless there is objection, I'll commit the following patch:

diff --git a/array.c b/array.c
index df0a0a4..481eebc 100644
--- a/array.c
+++ b/array.c
@@ -2605,12 +2605,12 @@ rb_ary_keep_if(VALUE ary)

/*

• call-seq:

• • ary.delete(obj)            -> obj or nil
    

• • ary.delete(obj) { block }  -> obj or nil
    

• • ary.delete(obj)            -> element or nil
    

• • ary.delete(obj) { block }  -> element or result of block
    

  • Deletes all items from +self+ that are equal to +obj+.

• • If any items are found, returns +obj+, otherwise +nil+ is returned instead.

• • Returns the last deleted item, or +nil+ if no matching item is found.
  • If the optional code block is given, the result of the block is returned if
  • the item is not found. (To remove +nil+ elements and get an informative
    diff --git a/test/ruby/test_array.rb b/test/ruby/test_array.rb
    index 8d264d9..6466fc3 100644
    --- a/test/ruby/test_array.rb
    +++ b/test/ruby/test_array.rb
    @@ -598,6 +598,14 @@ class TestArray < Test::Unit::TestCase
    a = @cls[('cab'..'cat').to_a]
    assert_equal(99, a.delete('cup') { 99 } )
    assert_equal(@cls[('cab'..'cat').to_a], a)
• o = Object.new
• def o.==(other); true; end
• o2 = Object.new
• def o2.==(other); true; end
• a = @cls[1, o, o2, 2]
• assert_equal(o2, a.delete(42))
• assert_equal([1, 2], a)
  end

def test_delete_at

[mame (Yusuke Endoh)]

• Status changed from Open to Assigned
• Assignee set to marcandre (Marc-Andre Lafortune)

[headius (Charles Nutter)]

marcandre (Marc-Andre Lafortune) wrote:

Indeed, the documentation does not match the code and there is no test for this.

It was clearly Matz' intention to return the (last) deleted element: https://bugs.ruby-lang.org/projects/ruby-trunk/repository/revisions/18527

This is the most helpful behavior anyways.

I'll note that the documentation is clearly wrong in the second form too.

Unless there is objection, I'll commit the following patch:

Yes, it appears to have been an explicit behavioral change in the 1.9.1/1.8.7 timeframe that never got reflected in documentation.

[marcandre (Marc-Andre Lafortune)]

• Status changed from Assigned to Closed

Documentation fixed, thanks for bringing this up.