Forum: IronRuby Review: fixes for Array#rindex and Array#reverse_each

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
Daniele A. (Guest)
on 2009-04-23 23:59
(Received via mailing list)
Attachment: specs_core_array_090423.diff (0 Bytes)
Hi,
I just pushed two fixes on my repository, the first one addresses a
bug in Array#rindex (there was a bug in my last commit) and the second
one makes Array#reverse_each compliant with the rubyspecs.

http://github.com/nrk/ironruby/commit/d2b18f5d01a4...

>From the commit message:

* Fixed a bug in IListOps.ReverseIndex (core/array/rindex specs were
passing, this bug was triggered under certain conditions different from
the ones defined in the specs)

* Fixed ArrayOps.ReverseEach to make it not fail when elemens in the
array
are removed from inside a block.

See also the attached diff.

Thanks,
Daniele
Jim D. (Guest)
on 2009-04-24 00:14
(Received via mailing list)
Can you add specs for rindex that expose the bug you fixed? Also, is
there any shared place that you could put the following code:
         if (self.Count < originalSize) {
             i = originalSize - i - 1 + self.Count;
             originalSize = self.Count;
         }

It would be nice to get rid of the duplicated logic, but I can't think
of where it should go.

Other than that, looks good.
JD
Daniele A. (Guest)
on 2009-04-24 01:04
(Received via mailing list)
Ha! I got caught :-) The duplicated logic is there precisely because
I, too, couldn't think of a place to share that small piece of code in
a good way, so I just coded it like that for now but I'll ponder about
that for the next commit. OK for the specs.

Thanks,
Daniele

On Thu, Apr 23, 2009 at 22:08, Jim D. <removed_email_address@domain.invalid>
wrote:
>
>> in Array#rindex (there was a bug in my last commit) and the second one
>>
>> http://www.clorophilla.net/blog/
>> http://twitter.com/JoL1hAHN
> _______________________________________________
> Ironruby-core mailing list
> removed_email_address@domain.invalid
> http://rubyforge.org/mailman/listinfo/ironruby-core
>



--
Daniele A.
http://www.clorophilla.net/blog/
http://twitter.com/JoL1hAHN
Daniele A. (Guest)
on 2009-05-31 15:23
(Received via mailing list)
Attachment: patch_093105A.diff (0 Bytes)
Hi Jim,

I'm a bit late but daytime work got in the way and then I was out of
the country for two weeks :-)
I got rid of the duplicated logic in IListOps.ReverseIndex and
ArrayOps.ReverseEach by implementing a new internal method
(IListOps.ReverseEnumerateIndexes):

http://github.com/nrk/ironruby/commit/76db817d1766...

As for the specs, I just slightly modified an existing one in a way
that does not affect the test but helped me to discover the bug
(actually this change uses a different condition from the one I used
one month ago as it exposed another bug which got fixed in the above
mentioned commit):

http://github.com/nrk/ironruby/commit/e5497cf87fc4...

See also the attached diff.

Thanks,
Daniele
Daniele A. (Guest)
on 2009-06-25 00:36
(Received via mailing list)
Hi,

It seems like this patch has gone unnoticed as it didn't get any
review yet so here I am, requesting for one :-)

Thanks,
Daniele

On Sun, May 31, 2009 at 13:22, Daniele A.<removed_email_address@domain.invalid>
wrote:
> As for the specs, I just slightly modified an existing one in a way
> Daniele
>>
>>>
>>> * Fixed a bug in IListOps.ReverseIndex (core/array/rindex specs were
>>>
>
>
> --
> Daniele A.
> http://www.clorophilla.net/blog/
> http://twitter.com/JoL1hAHN
>



--
Daniele A.
http://www.clorophilla.net/blog/
http://twitter.com/JoL1hAHN
Jim D. (Guest)
on 2009-06-25 02:31
(Received via mailing list)
Looks good. Nice job combining the logic. I think I went over this
patch, but never sent a review mail, sorry about that!

I'll pull it in with the next pull.

JD

…there is no try
Daniele A. (Guest)
on 2009-06-25 03:29
(Received via mailing list)
Thanks Jim!

BTW currently my repository is about one month behind. I must merge it
with
the main repository but I can't do that now, I hope to make it by
tomorrow
in the late afternoon.
Jim D. (Guest)
on 2009-06-25 03:30
(Received via mailing list)
Should be fine. I don’t plan on doing a pull until Friday.

From: removed_email_address@domain.invalid
[mailto:removed_email_address@domain.invalid] On Behalf Of Daniele
Alessandri
Sent: Wednesday, June 24, 2009 4:23 PM
To: removed_email_address@domain.invalid
Subject: Re: [Ironruby-core] Review: fixes for Array#rindex and
Array#reverse_each

Thanks Jim!

BTW currently my repository is about one month behind. I must merge it
with the main repository but I can't do that now, I hope to make it by
tomorrow in the late afternoon.


On Wed, Jun 24, 2009 at 23:42, Jim D.
<removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>> 
wrote:
Looks good. Nice job combining the logic. I think I went over this
patch, but never sent a review mail, sorry about that!

I'll pull it in with the next pull.

JD

…there is no try


> It seems like this patch has gone unnoticed as it didn't get any review yet so
> > the country for two weeks :-) I got rid of the duplicated logic in
> > that does not affect the test but helped me to discover the bug
> >
> >>         }
> >>> Sent: Thursday, April 23, 2009 12:59 PM
> http://github.com/nrk/ironruby/commit/d2b18f5d01a4...
> >>> array are removed from inside a block.
> >> _______________________________________________
> > http://twitter.com/JoL1hAHN
> removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>
> http://rubyforge.org/mailman/listinfo/ironruby-core
_______________________________________________
Ironruby-core mailing list
removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>
http://rubyforge.org/mailman/listinfo/ironruby-core



--
Daniele A.
http://www.clorophilla.net/blog/
http://twitter.com/JoL1hAHN
Daniele A. (Guest)
on 2009-06-26 00:52
(Received via mailing list)
Just merged and pushed to my remote repository.

On Thu, Jun 25, 2009 at 01:29, Jim D.<removed_email_address@domain.invalid>
wrote:
> Array#reverse_each
>
>
>> Array#reverse_each
>>
>> >
>> >
>> > On Thu, Apr 23, 2009 at 22:08, Jim D. <removed_email_address@domain.invalid>
>> >> of
>> >>> Subject: [Ironruby-core] Review: fixes for Array#rindex and
>> >>> 3ac94e0
>> >>> See also the attached diff.
>> >> removed_email_address@domain.invalid
>>
> _______________________________________________
> _______________________________________________
> Ironruby-core mailing list
> removed_email_address@domain.invalid
> http://rubyforge.org/mailman/listinfo/ironruby-core
>
>



--
Daniele A.
http://www.clorophilla.net/blog/
http://twitter.com/JoL1hAHN
This topic is locked and can not be replied to.