# \[patch\] More fixes for Array (IListOps.cs)

**URL:** <https://www.ruby-forum.com/t/patch-more-fixes-for-array-ilistops-cs/165611>\
**Category:** IronRuby\
**Created:** [April 15, 2009, 6:36pm UTC](https://www.ruby-forum.com/t/patch-more-fixes-for-array-ilistops-cs/165611 "2009-04-15T18:36:10Z")\
**Posts on this page:** 7\
**Page:** 1

<div class="post-metadata">

**Author:** ![murena](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/m/7ba0ec/32.png) [@murena](https://www.ruby-forum.com/u/murena)\
**Post date:** [April 15, 2009, 6:36pm UTC](https://www.ruby-forum.com/t/patch-more-fixes-for-array-ilistops-cs/165611/1 "2009-04-15T18:36:10Z")

</div>

Hi,

I’ve committed more fixes for a few methods of the Array class that  
were failing to pass the specs:

> <https://github.com/nrk/ironruby/commit/b754ddcd3bdd2c4513e456c9cf3933772e8541f9>
>
> \* The result of Array#compact must keep the tainted status of the source array.
> …
> \* A block passed to Array#delete should be executed only if no element matches the given object
> 
> \* Array#flatten returns subclass instance for Array subclasses
> 
> \* Array#flatten does not call flatten on elements
> 
> \* Array#fetch passes the original index argument object to the block, not the converted Integer

These are the specs affected by this patch:

- core/array/compact
  - Array#compact keeps tainted status even if all elements are removed

- core/array/delete
  - Array#delete may be given a block that is executed if no element  
matches object

- core/array/first
  - Array#first does not return subclass instance when passed count on  
Array subclasses

- core/array/last
  - Array#last does not return subclass instance on Array subclasses

- core/array/flatten
  - Array#flatten returns subclass instance for Array subclasses
  - Array#flatten does not call flatten on elements

- core/array/fetch
  - Array#fetch passes the original index argument object to the  
block, not the converted Integer

I hope I haven’t forgotten anything important this time, changes to  
the tags files are included in the commit 🙂

Regards,  
Daniele

–  
Daniele A.  
[http://www.clorophilla.net/blog/](http://www.clorophilla.net/blog/)  
[http://twitter.com/JoL1hAHN](http://twitter.com/JoL1hAHN)

---

<div class="post-metadata">

**Author:** ![murena](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/m/7ba0ec/32.png) [@murena](https://www.ruby-forum.com/u/murena)\
**Post date:** [April 16, 2009, 1:43am UTC](https://www.ruby-forum.com/t/patch-more-fixes-for-array-ilistops-cs/165611/2 "2009-04-16T01:43:05Z")

</div>

In IListOps.Fetch, instead of doing the protocol conversion manually,  
you should calls Protocols. ConvertToInteger. Tomas, can you  
double-check this?

In IListOps.First, you can get rid of the allocateStorage argument as  
its not being used anymore. (You can batch this fix with your next  
commit)

The rest looks good! Great to see array getting more compliant!

Btw, could you please increase the diff context size to 15-20 so more of  
the surrounding lines are included? That makes it easier to understand  
what the fix is doing when using the url below, without having to map  
that back to the correct line number in the actual file.

Shri

---

<div class="post-metadata">

**Author:** ![murena](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/m/7ba0ec/32.png) [@murena](https://www.ruby-forum.com/u/murena)\
**Post date:** [April 16, 2009, 6:15am UTC](https://www.ruby-forum.com/t/patch-more-fixes-for-array-ilistops-cs/165611/3 "2009-04-16T06:15:18Z")

</div>

Yes, Shri is right in essence - you shouldn’t do protocol conversions  
manually. The conversion used by DefaultProtocol on int is  
Protocols.CastToInteger however (ConvertToInteger performs different  
integer conversion).

Tomas

From: removed\_email\_address@domain.invalid  
[[mailto:removed\_email\_address@domain.invalid](mailto:removed_email_address@domain.invalid)] On Behalf Of Shri B.  
Sent: Wednesday, April 15, 2009 4:14 PM  
To: removed\_email\_address@domain.invalid  
Subject: Re: [Ironruby-core] [patch] More fixes for Array (IListOps.cs)

In IListOps.Fetch, instead of doing the protocol conversion manually,  
you should calls Protocols. ConvertToInteger. Tomas, can you  
double-check this?

In IListOps.First, you can get rid of the allocateStorage argument as  
its not being used anymore. (You can batch this fix with your next  
commit)

The rest looks good! Great to see array getting more compliant!

Btw, could you please increase the diff context size to 15-20 so more of  
the surrounding lines are included? That makes it easier to understand  
what the fix is doing when using the url below, without having to map  
that back to the correct line number in the actual file.

Shri

---

<div class="post-metadata">

**Author:** ![murena](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/m/7ba0ec/32.png) [@murena](https://www.ruby-forum.com/u/murena)\
**Post date:** [April 16, 2009, 11:33am UTC](https://www.ruby-forum.com/t/patch-more-fixes-for-array-ilistops-cs/165611/4 "2009-04-16T11:33:34Z")

</div>

On Thu, Apr 16, 2009 at 01:14, Shri B. [removed\_email\_address@domain.invalid](mailto:removed_email_address@domain.invalid)  
wrote:

> In IListOps.Fetch, instead of doing the protocol conversion manually, you  
> should calls Protocols. ConvertToInteger. Tomas, can you double-check this?

Hmm, right, and the code looks much more cleaner too.  
I think I should just use Protocols.CastToFixnum then: we don’t really  
care about bignums here, they can’t be used as indexes for arrays  
anyway and in fact MRI throws a RangeError “bignum too big to convert  
into `long’”.

> In IListOps.First, you can get rid of the allocateStorage argument as its  
> not being used anymore. (You can batch this fix with your next commit)

Same for IListOps.Last, it just slipped away. I removed allocateStorage  
on both.

> Btw, could you please increase the diff context size to 15-20 so more of the  
> surrounding lines are included? That makes it easier to understand what the  
> fix is doing when using the url below, without having to map that back to  
> the correct line number in the actual file.

I guess there is no way to increase the lines range of a diff viewed  
via the github web interface, but I can generate and attach custom  
diff patches to my mails if you think it could be useful, just like  
the one attached to this mail and generated with "git diff HEAD~1 -U15

> patch.diff" (PS: I have not yet pushed this commit on my remote  
> repository).

---

<div class="post-metadata">

**Author:** ![murena](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/m/7ba0ec/32.png) [@murena](https://www.ruby-forum.com/u/murena)\
**Post date:** [April 16, 2009, 7:48pm UTC](https://www.ruby-forum.com/t/patch-more-fixes-for-array-ilistops-cs/165611/5 "2009-04-16T19:48:27Z")

</div>

Result of “git diff HEAD~1 -U15” would be nice if you can remember to  
include it, but donâ€™t worry about it too much. I will try to deal with  
the [github.com](http://github.com) web view and see if it bothers me too much.

---

<div class="post-metadata">

**Author:** ![murena](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/m/7ba0ec/32.png) [@murena](https://www.ruby-forum.com/u/murena)\
**Post date:** [April 16, 2009, 7:29pm UTC](https://www.ruby-forum.com/t/patch-more-fixes-for-array-ilistops-cs/165611/6 "2009-04-16T19:29:38Z")

</div>

Correction: CastToFixnum is the right one. Ruby has just to many integer  
conversions to remember 🙂

Tomas

From: removed\_email\_address@domain.invalid  
[[mailto:removed\_email\_address@domain.invalid](mailto:removed_email_address@domain.invalid)] On Behalf Of Tomas M.  
Sent: Wednesday, April 15, 2009 8:45 PM  
To: removed\_email\_address@domain.invalid  
Subject: Re: [Ironruby-core] [patch] More fixes for Array (IListOps.cs)

Yes, Shri is right in essence - you shouldn’t do protocol conversions  
manually. The conversion used by DefaultProtocol on int is  
Protocols.CastToInteger however (ConvertToInteger performs different  
integer conversion).

Tomas

From: removed\_email\_address@domain.invalid  
[[mailto:removed\_email\_address@domain.invalid](mailto:removed_email_address@domain.invalid)] On Behalf Of Shri B.  
Sent: Wednesday, April 15, 2009 4:14 PM  
To: removed\_email\_address@domain.invalid  
Subject: Re: [Ironruby-core] [patch] More fixes for Array (IListOps.cs)

In IListOps.Fetch, instead of doing the protocol conversion manually,  
you should calls Protocols. ConvertToInteger. Tomas, can you  
double-check this?

In IListOps.First, you can get rid of the allocateStorage argument as  
its not being used anymore. (You can batch this fix with your next  
commit)

The rest looks good! Great to see array getting more compliant!

Btw, could you please increase the diff context size to 15-20 so more of  
the surrounding lines are included? That makes it easier to understand  
what the fix is doing when using the url below, without having to map  
that back to the correct line number in the actual file.

Shri

---

<div class="post-metadata">

**Author:** ![murena](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/m/7ba0ec/32.png) [@murena](https://www.ruby-forum.com/u/murena)\
**Post date:** [April 16, 2009, 10:18pm UTC](https://www.ruby-forum.com/t/patch-more-fixes-for-array-ilistops-cs/165611/7 "2009-04-16T22:18:22Z")

</div>

There is a setting in “git gui” I was thinking of, but it does not  
affect the github web view. Never mind…

From: Jim D.  
Sent: Wednesday, April 15, 2009 5:14 PM  
To: Shri B.  
Subject: RE: [Ironruby-core] [patch] More fixes for Array (IListOps.cs)

There is no control over the context size on Github… Unless I  
misunderstood your point

From: removed\_email\_address@domain.invalid  
[[mailto:removed\_email\_address@domain.invalid](mailto:removed_email_address@domain.invalid)] On Behalf Of Shri B.  
Sent: Wednesday, April 15, 2009 4:14 PM  
To: removed\_email\_address@domain.invalid  
Subject: Re: [Ironruby-core] [patch] More fixes for Array (IListOps.cs)

In IListOps.Fetch, instead of doing the protocol conversion manually,  
you should calls Protocols. ConvertToInteger. Tomas, can you  
double-check this?

In IListOps.First, you can get rid of the allocateStorage argument as  
its not being used anymore. (You can batch this fix with your next  
commit)

The rest looks good! Great to see array getting more compliant!

Btw, could you please increase the diff context size to 15-20 so more of  
the surrounding lines are included? That makes it easier to understand  
what the fix is doing when using the url below, without having to map  
that back to the correct line number in the actual file.

Shri
