search

evalEmpire / perl5i

A single module to fix as much of Perl 5 as possible in one go
http://search.cpan.org/perldoc?perl5i
Other
156 stars 42 forks source link

@array->join should default to something #241

Open notbenh opened 11 years ago

notbenh commented 11 years ago

Currently @array->join warns a bunch but because join undef actually works it does what I expected. So should join default to q{}? or should we make a slightly more useful default on this one? A single space would be nice and useful in more cases then join(q{}).

schwern commented 11 years ago

My impulse is to make @a->join an exception on the philosophy that what YOU expect and what I expect and what the reader six months down the road expects are different and @a->join("") isn't a big trouble.

That you might want the default to be something other than empty string just underlines the point.

notbenh commented 11 years ago

This is starting to get complex, I am going to split this out in to separate issues:

what ~could~ the default be is now #244 what ~should~ happen with bare method calls is now #245

Back to the topic at hand:

You are correct that I have phrased this in a less than descriptive manner. Currently ->join does default to something, err rather nothing, more specifically undef. Thus the issue is that you get a warning about use of undef but it ~works~ as in you get the value back. For the sake of illustration: 

> perl5i -e 'print [1,2,3]->join'
Use of uninitialized value $sep in join or string at /home/benh/perl5/lib/perl5/autobox/Core.pm line 1625.
123

and here is the code in question from /home/benh/perl5/lib/perl5/autobox/Core.pm (where ever that is built from)

sub join { my $arr = CORE::shift; my $sep = CORE::shift; CORE::join $sep, @$arr; }

So to take another attempt at clarifying my point, it seems like a poor interface to allow for something supply warnings and still preform a reasonable action. This could be solved by supplying a value to $sep. The current action would be maintained, sans warnings, by defaulting to q{}. Thus do we want to do this to solve the issue with the warning?

schwern commented 11 years ago

If I understand what you're saying, you're concerned with preserving the current behavior? That's why changing it to throw an exception is not ok? Normally, I'd agree, and it is correct procedure.

However, @a->join throws a warning every time. Either the user is ignoring the warning (on by default) OR they heeded the warning and changed their code to pass in an argument. Furthermore, every use of join in the autobox::Core docs uses an explicit separator. Finally, the built in join which it is emulating has no default.

Because of all that, I'd say its highly unlikely @a->join is being used in production and its safe to change the behavior to be an exception without a major version change.