How can I sort one key ascending, another descending without using $b (comparison) $a in Perl?

Question

In Perl, I can do:

my @unit_indices = sort { 
    $units{$b}[0] <=> $units{$a}[0] 
        or
    $a cmp $b
} keys %units;

which sorts by one field (array element) descending and the other (hash key) ascending, but causes perlcritic to complain:

Forbid $b before $a in sort blocks at line X, column Y. See page 152 of PBP. (Severity: 1)

Perl Best Practices recommends using reverse instead.

But the operation would be much more comprehensible if you wrote:
@sorted_results = reverse sort @unsorted_results;

However, I haven't found a way to have subsorts that run in opposite directions.

Obviously, I can tell perlcritic to ignore this, but I'd like to know how to accomplish what I need and make perlcritic happy.

Why in the world would you use the asquerous `perlcritic`??? It’s incredibly stooopeeed. That’s your first mistake. Second, you’ve confused the PBP advice. Your initial sort function is fine, except I would of course eschew `or` in lieu of `||` to improve legibility. Your question is mistitled, because you clearly know the right way to do it already. — tchrist, Feb 22 '12 at 16:33
Your code looks fine. Sounds like a bug in perlcritic - $a and $b are valid perl variables for sorting. It should only warn if $a and $b exist outside a sort block for that scope and any blocks inside the loop. Looks like it doesn't detect multiple key sorts. — Rich, Feb 22 '12 at 16:47
A similar issue has been reported https://rt.cpan.org/Public/Bug/Display.html?id=36129 — toolic, Feb 22 '12 at 18:02
@Schwern I *am* being nice. I very much believe that Perl Critic does more harm than good. Too much unthinking obsequious idolatry of a false idol. And I hate the bludgeoning uses that Perl Best Practices has been put to almost as much as does Damian himself. People misunderstood the whole point of that book. — tchrist, Feb 22 '12 at 20:20
@tchrist: I take the point to be that one should have coding standards - here's a set to start from. I don't take it to be the last word to be used mindlessly. — Dennis Williamson, Feb 22 '12 at 21:23
@DennisWilliamson Yes, that’s right. There’s a story out there of a Perl Pumpking who while working as a contractor got outrageously dinged on a code review because his utterly reasonable and correct code made the stupid Perl Critic emit strange smells and noises. The people didn’t understand the complaints, or perl, just that they knew the Corporation required that all code sneak past Perl Genuflect-When-Say-That Critic without a peep. It’s outrageous. — tchrist, Feb 23 '12 at 02:16

score 9 · Answer 1 · answered Feb 22 '12 at 20:41

Perl::Critic at times will concern itself more with being an accurate reproduction of PBP than presenting good policy, and some of Perl Best Practices hasn't aged well. For example, the now woefully out of date Miscellanea::RequireRcsKeywords is still on by default.

Perl::Critic's policies shouldn't be treated as canon. They lack the ability to do subjective analysis to decide if the "fix" is actually going to increase complexity, particularly as the severity level drops and the benefits get narrower and narrower. BuiltinFunctions::ProhibitReverseSortBlock is a "cosmetic" level policy and falls squarely into this category.

While somebody might skip $b cmp $a and read it backwards, it is not difficult to comprehend once squarely looked at, is not worth the overhead of reversing the whole array afterward, and is certainly not worth contorting your sort block to match the limitations of the policy analysis. Their decision not to change the default behavior to limit the policy to simple sort blocks is IMO incorrect. Your sort block is obviously beyond the scope of the written policy and is only triggered because Perl::Critic's policy implementation is limited.

Just because Perl::Critic has a policy on by default doesn't mean they represent good practice nor that they should be followed blindly. Feel free to configure it to your project's tastes, running Perl::Critic at its most picky levels requires it. In order to prevent silencing perlcritic from being a habitual thing, I would recommend preferring project-wide policy decisions in .perlcritic to individually turning them off line by line.

Remember, the point is not to be perlcritic happy, the point is to write better code.

+1. I agree. The point of my question, which I should have made clear, was that I wanted to know what the alternative was so I could make an informed decision regarding selectively ignoring the warning. — Dennis Williamson, Feb 22 '12 at 21:20

score 4 · Accepted Answer · answered Feb 22 '12 at 16:47

4

I agree with tchrist's comment, but taking your question at face value:

1) use ## no critic

my @unit_indices = sort {   ## no critic (ReverseSortBlock)
    $units{$b}[0] <=> $units{$a}[0] 
        or
    $a cmp $b
} keys %units;

2) Use a .perlcriticrc file

[-BuiltinFunctions::ProhibitReverseSortBlock]

3) Change the sense of your comparison

my @unit_indices = sort { 
    -($units{$a}[0] <=> $units{$b}[0])
        or
    $a cmp $b
} keys %units;

my @unit_indices = sort { 
    -$units{$a}[0] <=> -$units{$b}[0])
        or
    $a cmp $b
} keys %units;

answered Feb 22 '12 at 16:47

mob

117,087
18
149
283

5

Ick. Don't do 3, that just makes your code harder to understand for real people who aren't robots. – cjm Feb 22 '12 at 16:57
@cjm - does that mean I'm a robot? :) – DVK Feb 22 '12 at 17:16
1

We need a `>=<` operator in Perl 5.16! – mob Feb 22 '12 at 17:29
1

I'd vote for a `>_<` operator, or perhaps `o.O` (functionality TBD). – Dennis Williamson Feb 22 '12 at 17:48
1

+1. As I said in my question, I know I can tell `perlcritic` to ignore it. Your item 3 is what I was looking for. I agree with @cjm that it's not the right thing to do. Having seen your answer, I now can make the informed choice to add the directive to `.perlcriticrc`. Thanks. – Dennis Williamson Feb 22 '12 at 21:29
@mob, conflicts with `>= <$fh>` – ikegami Feb 22 '12 at 22:31

ikegami · Answer 3 · 2012-02-23T20:34:49.923

2

Feel free to use perlcritic, but don't be a slave to it. The whole point of having perlcritic is to be have the ability to issue warnings that have a good chance of being wrong.

It's surely trying to encourage the use of reverse sort { $a <=> $b } over sort { $b <=> $a }, but that can't be done here.

Don't make your code worse to silence a spurious warning. Sometimes, it's the warning that's wrong, in which case you should address the warning.

edited Feb 23 '12 at 20:34

answered Feb 22 '12 at 22:34

ikegami

367,544
15
269
518

Do *you* use `perlcritic`? I would be rather surprised to learn that. – tchrist Feb 23 '12 at 02:18
@tchrist, I suppose I meant "Feel free to use perlcritic" instead of "Use perlcritic". (Fixed) I tried to create a profile at work (mostly for the benefit of my coworkers), but it began to feel like it was a waste of time. Just too many bad calls. I can't imagine someone trying to corrupt their code to silence them all. – ikegami Feb 23 '12 at 20:32
3

I find the idea of perlcritic worthwhile. It looks over my shoulder and keeps me honest. Over the years I have molded it to my will and commented my rationales. It might be a better place to start. https://github.com/schwern/test-more/blob/Test-Builder1.5/.perlcriticrc – Schwern Feb 23 '12 at 21:00
@Schwern, Thanks, I'll have a look. It might be interesting simply to see what you find important. I guess that's the reason I tried to run it in the first place. – ikegami Feb 23 '12 at 21:04

How can I sort one key ascending, another descending without using $b (comparison) $a in Perl?

3 Answers3