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.
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.
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.
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.
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):
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):
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.
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.
<[email protected]mailto:[email protected]> 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
The duplicated logic is there precisely because