Bug #1367
closed
flatten(0) is not consistent with flatten(), flatten(1), etc.
Description
=begin
a = [1, 2]
a2 = a.flatten
a2 << :a
a2
=> [1, 2, :a]
a
=> [1, 2]
b = [3, 4]
b2 = b.flatten 1
b2 << :b
b2
=> [3, 4, :b]
b
=> [3, 4]
HOWEVER:
c = [5, 6]
c2 = c.flatten 0
c2 << :c
c2
=> [5, 6, :c]
c
=> [5, 6, :c] <-- Huh? Should be [5, 6].
=end
Updated by schouery (Rafael Schouery) about 15 years ago
=begin
I did take a look in the problem:
static VALUE
rb_ary_flatten(int argc, VALUE *argv, VALUE ary)
{
int mod = 0, level = -1;
VALUE result, lv;
rb_scan_args(argc, argv, "01", &lv);
if (!NIL_P(lv)) level = NUM2INT(lv);
if (level == 0) return ary;
result = flatten(ary, level, &mod);
OBJ_INFECT(result, ary);
return result;
}
In the line
if (level == 0) return ary;
you will get the reference to the original array, instead of
a reference to a clone.
The problem is: this is a bug or a feature? =D
That is, what is the behavior that we want when calling
flatten with 0, since this call doesn't make much sense.
It is easy to fix it, but I don't know if I should. If you guys
think it is a good idea, that I can take some time to do that.
Bye!
Rafael Schouery
VidaGeek.net
On Wed, Apr 8, 2009 at 8:01 PM, Paul Lewis redmine@ruby-lang.org wrote:
Bug #1367: flatten(0) is not consistent with flatten(), flatten(1), etc.
http://redmine.ruby-lang.org/issues/show/1367Author: Paul Lewis
Status: Open, Priority: Normal
Category: core
ruby -v: ruby 1.9.0 (2008-06-20 revision 17482) [i486-linux]a = [1, 2]
a2 = a.flatten
a2 << :a
a2
=> [1, 2, :a]
a
=> [1, 2]b = [3, 4]
b2 = b.flatten 1
b2 << :b
b2
=> [3, 4, :b]
b
=> [3, 4]HOWEVER:
c = [5, 6]
c2 = c.flatten 0
c2 << :c
c2
=> [5, 6, :c]
c
=> [5, 6, :c] <-- Huh? Should be [5, 6].
I did take a look in the problem:
static VALUE
rb_ary_flatten(int argc, VALUE *argv, VALUE ary)
{
int mod = 0, level = -1;
VALUE result, lv;
rb_scan_args(argc, argv, "01", &lv);
if (!NIL_P(lv)) level = NUM2INT(lv);
if (level == 0) return ary;
result = flatten(ary, level, &mod);
OBJ_INFECT(result, ary);
return result;
}
In the line
if (level == 0) return ary;
you will get the reference to the original array, instead of
a reference to a clone.
The problem is: this is a bug or a feature? =D
That is, what is the behavior that we want when calling
flatten with 0, since this call doesn't make much sense.
It is easy to fix it, but I don't know if I should. If you guys
think it is a good idea, that I can take some time to do that.
Bye!
Rafael Schouery
VidaGeek.net
On Wed, Apr 8, 2009 at 8:01 PM, Paul Lewis <redmine@ruby-lang.org> wrote:
> Bug #1367: flatten(0) is not consistent with flatten(), flatten(1), etc.
> http://redmine.ruby-lang.org/issues/show/1367
>
> Author: Paul Lewis
> Status: Open, Priority: Normal
> Category: core
> ruby -v: ruby 1.9.0 (2008-06-20 revision 17482) [i486-linux]
>
>> a = [1, 2]
>> a2 = a.flatten
>> a2 << :a
>> a2
> => [1, 2, :a]
>> a
> => [1, 2]
>
>> b = [3, 4]
>> b2 = b.flatten 1
>> b2 << :b
>> b2
> => [3, 4, :b]
>> b
> => [3, 4]
>
> HOWEVER:
>
>> c = [5, 6]
>> c2 = c.flatten 0
>> c2 << :c
>> c2
> => [5, 6, :c]
>> c
> => [5, 6, :c] <-- Huh? Should be [5, 6].
>
>
> ----------------------------------------
> http://redmine.ruby-lang.org
>
>
=end
Updated by pjlewis (Paul Lewis) about 15 years ago
=begin
As a pro-fix argument consider what happens when the code using flatten has a dynamic depth value. So rather than:
x = a.flatten(0)
It's something like:
x = a.flatten(depth)
To ensure that this code behaviour predictably (without this fixed) it becomes necessary to treat 0 as a special case:
depth = ???
x = (depth != 0 ? a.flatten() : a.clone())
As it works at the moment I think it violates the principle of least surprise.
http://en.wikipedia.org/wiki/Principle_of_least_surprise
=end
Updated by mame (Yusuke Endoh) about 15 years ago
- Status changed from Open to Closed
- % Done changed from 0 to 100
=begin
Applied in changeset r23191.
=end
Updated by schouery (Rafael Schouery) about 15 years ago
=begin
I thought about that too.
I was think about other functions. sort, of course, is going to give you a
new array,
but what about functions that already knows that is not going to do
anything, like
[].reverse ?
Ruby gives you a new array, so I think this is really a bug. I want to make
a patch
for it soon, but I don't know when I will be able to.
Bye!
Rafael Schouery
VidaGeek.net
On Wed, Apr 15, 2009 at 7:11 AM, Paul Lewis redmine@ruby-lang.org wrote:
Issue #1367 has been updated by Paul Lewis.
As a pro-fix argument consider what happens when the code using flatten has
a dynamic depth value. So rather than:x = a.flatten(0)
It's something like:
x = a.flatten(depth)
To ensure that this code behaviour predictably (without this fixed) it
becomes necessary to treat 0 as a special case:depth = ???
x = (depth != 0 ? a.flatten() : a.clone())As it works at the moment I think it violates the principle of least
surprise.
http://en.wikipedia.org/wiki/Principle_of_least_surprisehttp://redmine.ruby-lang.org/issues/show/1367
I thought about that too.
I was think about other functions. sort, of course, is going to give you a new array,
but what about functions that already knows that is not going to do anything, like
[].reverse ?
Ruby gives you a new array, so I think this is really a bug. I want to make a patch
for it soon, but I don't know when I will be able to.
Bye!
Rafael Schouery
VidaGeek.net
Issue #1367 has been updated by Paul Lewis.
----------------------------------------
As a pro-fix argument consider what happens when the code using flatten has a dynamic depth value. So rather than:
x = a.flatten(0)
It's something like:
x = a.flatten(depth)
To ensure that this code behaviour predictably (without this fixed) it becomes necessary to treat 0 as a special case:
depth = ???
x = (depth != 0 ? a.flatten() : a.clone())
As it works at the moment I think it violates the principle of least surprise.
http://en.wikipedia.org/wiki/Principle_of_least_surprise
----------------------------------------
=end
Updated by mame (Yusuke Endoh) about 15 years ago
=begin
Hi,
2009/4/15 Rafael Crivellari Saliba Schouery schouery@gmail.com:
Ruby gives you a new array, so I think this is really a bug. I want to make
a patch
for it soon, but I don't know when I will be able to.
Thank you, but I've just fixed in r23191.
--
Yusuke ENDOH mame@tsg.ne.jp
=end