Re: Code review

That indented line could be written as follows:

++$Y{$y} if grep { $y eq $_ } @ARGV;
[download]

It does loose the short-circuiting aspect, but that's not likely to be relevant. But what if we take it a step further.

my $re;
if ( @ARGV ) {
   my $pat = join "|", map quotemeta, @ARGV;
   $re = qr/^(?:$pat)\z/;
}

my %Y;
while ( <> ) {
  my ( $y ) = ...;

  ++$Y{ $y } if $y && ( !$re || $y =~ $re );
}
[download]

Update: Hash simpler than regex.

my $re;
my %valid;
++$valid{ $_ } for @ARGV;

my %Y;
while ( <> ) {
  my ( $y ) = ...;

  ++$Y{ $y } if $y && ( !%valid || $valid{ $_ } );
}
[download]

Comment on Re: Code review Select or Download Code

Replies are listed 'Best First'.
Re^2: Code review by hv (Prior) on Mar 06, 2024 at 17:04 UTC
~~The first version does not allow for empty `@ARGV`;~~ both versions do not allow for the possibility that `@ARGV` may have repeated elements. I think a short form that preserves all the current behaviour could look like this: `... next unless $y; $Y{$y} += @ARGV ? (grep $y eq $_, @ARGV) : 1;` [download] It would, though, be preferable to have some clue about what the code is intended to achieve: it is always possible that the behaviour on repeated elements is unintentional or irrelevant.	[reply] [d/l] [select]
Re^3: Code review by ikegami (Patriarch) on Mar 07, 2024 at 05:27 UTC
The first version does not allow for empty `@ARGV` That's not true. If you remove the `if`, sure, but noone said to do that.	[reply] [d/l] [select]
Re^4: Code review by hv (Prior) on Mar 07, 2024 at 11:43 UTC
You're quite right, my apologies for the error.	[reply]