Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

@array->join should default to something #241

Open
notbenh opened this Issue Mar 21, 2013 · 3 comments

Comments

Projects
None yet
2 participants
Contributor

notbenh commented Mar 21, 2013

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{}).

Contributor

schwern commented Mar 22, 2013

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.

Contributor

notbenh commented Mar 22, 2013

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?

Contributor

schwern commented Mar 22, 2013

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment