[ruby-talk:5716] Re: Array#insert

> From: [e-mail] [mailto:[e-mail]]
> >>>>> "Mark" == Mark Slagell <[e-mail]> writes:
> 
>     >> I would prefer that using -1 would result in:
>     >> 
>     >> [1, 2, 3, [4, 5]]
> 
>     Mark> But it muddies the semantics, right? "insert before" for
>     Mark> non-negative indices, "insert after" for negative.
> 
> Don't think of it as "insert before" or "insert after".  Think of it
> as "insert at".  

Very good discussion. Guy's right about better semantics, Mark about real
meaning of semantics, and Jim about proposing the way to understand how
negative indexes work.

While Jim's following discussion 

>     After ...
>         a.insert (index, value)
>     then ...
>         a[index] == value
> 

applies when value is just one parameter, it has to be extended when
Array#insert is called with multiple values. The idea is the same anyway:

After ...
         a.insert (index, val1, val2, val3)
then
         a[index]   == val1
         a[index+1] == val2
         a[index+2] == val3

> BTW, shouldn't the method be named "insert!" ?

I think I'm following matz ideas when I named it to be "insert" instead of
"insert!". And the reasoning is that currently the methods are not named
with exclamation mark (!) if 

1) the name clearly indicates the receiving object is going to change and/or
2) there's no corresponding method with implicit dup. 

In this case the name indicates the receiver is going to be changed, and
there's no !-version (just like Array#delete_at doesn't have exclamation
mark).

My understanding is that a method which changes receiver can be named
without !. But a method which does not change receiver can't be named with
!.
Also a method which has a non-receiver-mutating version too has to be named
with !.

I'm sure matz will correct me here, if I'm mistaken or the "rules" are
incorrect. I'm also willing to create insert *and* insert! if it's thought
to be necessary or the current way confusing. I don't feel so (yet :).

Maybe the name should Array#insert_at, like delete_at. What do you think?

Anyway, here's an update patch, which passes the following RubyUnit tests:

Index: array.c
===================================================================
RCS file: /home/cvs/ruby/array.c,v
retrieving revision 1.30
diff -u -r1.30 array.c
--- array.c     2000/10/10 07:03:15     1.30
+++ array.c     2000/10/20 18:54:38
@@ -617,6 +617,42 @@
     return argv[1];
 }
 
+static VALUE
+rb_ary_insert(argc, argv, ary)
+    int argc;
+    VALUE *argv;
+    VALUE ary;
+{
+    VALUE objects;
+    long pos;
+
+    if (argc < 2) {
+       rb_raise(rb_eArgError, "wrong # of arguments (%d when 2 required)",
argc);
+    }
+
+    if (TYPE(argv[0]) != T_FIXNUM) {
+      rb_raise(rb_eArgError, "Argument position is not a fixnum");
+    }
+
+    objects = rb_ary_new4(argc-1, argv+1);
+
+    pos = FIX2INT(argv[0]);
+    /* tweak negative positions to insert in correct place so that
+       following array is indexed properly
+
+        [12, 34, 45, 56]
+          0   1   2   3    positive indexes
+         -4  -3  -2  -1    negative indexes
+    */
+    if (pos < 0)
+      if (++pos == 0)
+        pos = RARRAY(ary)->len;
+
+    rb_ary_replace(ary, pos, 0, objects);
+
+    return ary;
+}
+
 VALUE
 rb_ary_each(ary)
     VALUE ary;
@@ -1615,6 +1651,7 @@
 
     rb_define_method(rb_cArray, "[]", rb_ary_aref, -1);
     rb_define_method(rb_cArray, "[]=", rb_ary_aset, -1);
+    rb_define_method(rb_cArray, "insert", rb_ary_insert, -1);
     rb_define_method(rb_cArray, "at", rb_ary_at, 1);
     rb_define_method(rb_cArray, "first", rb_ary_first, 0);
     rb_define_method(rb_cArray, "last", rb_ary_last, 0);





  def test_insert
    # Jim Weirich describes a  way to understand 
    # insert's semantics at [ruby-talk:5700]
    a = [1,2,3]
    assert_equal([[4, 5], 1, 2, 3],      a.dup.insert(0, [4,5]) )
    assert_equal([1, [4, 5], 2, 3],      a.dup.insert(1, [4,5]) )
    assert_equal([1, 2, [4, 5], 3],      a.dup.insert(2, [4,5]) )
    assert_equal([1, 2, 3, [4, 5]],      a.dup.insert(3, [4,5]) )
    assert_equal([1, 2, 3, nil, [4, 5]], a.dup.insert(4, [4,5]) )

    assert_equal([1, 4, 5, 2, 3],   a.dup.insert(1, 4, 5) )

    assert_equal([1, 2, 3, [4, 5]], a.dup.insert(-1, [4,5]) )
    assert_equal([1, 2, [4, 5], 3], a.dup.insert(-2, [4,5]) )
    assert_equal([1, [4, 5], 2, 3], a.dup.insert(-3, [4,5]) )
    assert_equal([[4, 5], 1, 2, 3], a.dup.insert(-4, [4,5]) )

    # I'm a little bit unsure if this should be an error
    assert_exception(IndexError) { a.dup.insert(-5, [4,5]) }

    assert_exception(ArgumentError) { a.dup.insert(1) }
    assert_exception(ArgumentError) { a.dup.insert([4,5], 1) }    

    # Jim Weirich test extended
    b = a.dup
    b.insert(1, 6)
    assert_equal([1, 6, 2, 3], b)
    b.insert(1, 5)
    assert_equal([1, 5, 6, 2, 3], b)
    b.insert(1, 4)
    assert_equal([1, 4, 5, 6, 2, 3], b)
    assert_equal(b, a.dup.insert(1,  4, 5, 6))

    b = a.dup.insert(1, 4, 5, 6)
    assert_equal(4, b[1]  )
    assert_equal(5, b[1+1])
    assert_equal(6, b[1+2])
  end