pop sort strangeness

[Brovnik]

Why can't I do my $max = pop sort @vals ? It gives a "Type of arg 1 to pop must be array (not sort)" error, but sort does return an array. Is it assuming scalar context or some similar strangeness ?

my @vals = (1,3,6,3,5,2,7,6,8,7,3,4,5,9);
my $max = pop sort @vals; # gives error

my @max = sort(@vals);
my $max = pop(@max); # works ok

print "Max is $max\n";

[merlyn]

Sort returns a list, not an array. An array is the container for a list. Pop needs to have a container so that it can remove an element.

If you want the last element of a list, you can use a literal slice:

my $max = (... list ...)[-1];

Also, if you're sorting numbers, you'll want to look into user-defined sorts. The built-in sort sorts as strings, which will mess up your day on numbers.

[dragonchild]

sort returns a list, not an array. pop has a prototype on it that requires an actual array (for various reasons). So, you will need to do the second one or something like:

my $max = (sort @vals)[-1];

[davido]

pop pulls one item off the array. sort returns a list. That list may be assigned to an array, or it may be kept as a list. You're keeping it as a list, in a sort of scratch storage. In that form, it's not popable. ;)

This will work:

my $max = pop @{[ sort @vals ]};

This works because you're turning the list returned by sort into an anonymous array, which is immediately dereferenced.

Either way, this is an inefficient solution, as it requires that the entire list be sorted each time you take a max. Use the List::Util max() function, or if you're going to roll your own, just do a linear search.

[Brovnik]

my $max = pop @{[ sort @vals ]};

is the answer I was looking for, thanks. I realise it is inefficient, just came across the error and didn't immediately twig the list/array issue.

[davido]

Indeed, it is very inefficient, by comparison to simply doing a linear search. First, you're sorting the list. That's O(N log N) for Perl's default sort type. Next, you're taking a copy of the list (yes, you're copying it) for the purpose of creating the anonymous array. That's about O(N). Next, you're dereferencing the array (a pretty quick action), and then you're popping something off the array (which is also pretty quick, but still is another step).

So what you've got is O( N + N log N ), when you could just have O(N). That's not so good. And as someone else already pointed out, sort @vals does a string sort, not a numeric sort, so 11 will be sorted next to 1 instead of next to 12. If you must use the sort routine, at least change it to doing a numeric sort:

my $max = pop @{[ sort { $a <=> $b } @vals ]};

And here's a linear search for max.

my $max = $vals[0];
$max = ( $_ > $max ) ? $_ : $max foreach @vals;

This version is a little terse, on purpose, to demonstrate that it's possible to do the linear search in just slightly more keystrokes than the sort pop method.

[Roy Johnson]

Others have told you how to get max, but their solutions don't remove the element from the array, which your first example suggests you want to do, though your "works ok" example doesn't do that. If you do want it removed:

(my $max, @vals) = reverse sort @vals;
# or sort {$b cmp $a} @vals
# in any case, the one to assign to the scalar has to come first

I had thought that this would work:

my $max = pop(@vals = sort(@vals));

, but Perl sees pop's argument as a "list assignment" rather than an array, so I can't give you a one-line solution using pop.

On the other hand, if you don't want to modify @vals at all, it would make more sense to just do a max-finding search (an order-N task) rather than sorting the list (an order N*log(N) task).

use List::Util 'maxstr';
my $max = maxstr(@vals);

[Roy Johnson]

I came up with a one-line pop solution:

my $max = pop @{@vals=sort @vals; \@vals};

It's kind of cheating in the "one-line" department, and I don't recommend it as clear programming, but it illustrates that "a BLOCK returning a reference" really means a BLOCK, not just an expression in curlies. Obviously the clear way to write it would be

@vals = sort @vals;
my $max = pop @vals;

[Zaxo]

You could pop from an anonymous array like this, my $max = pop @{[sort @vals]}; where the cast turns sort's list into a nameless array. It's much better to say,

use List::Util qw(max);
my $max = max @vals;

There is a lot of lost motion in that unnecessary sort.

[Eimi Metamorphoumai]

Others have explained why that doesn't work, but I'll add that if all you want is the the maximum element, you're wasting a lot of time sorting the list to get it. I would suggest max or maxstr from List::Util for that.