EDAVIS via RT wrote:
Show quoted text> Sorry I gave an oversimplified example. I meant something more like
>
> if (/\A(foo|bar)\z/ or /\Afred:(foo|baz)\z/) {
> say "got $1";
> }
> else {
> die "malformed line: $_";
> }
>
> Currently perlcritic would have that rewritten with a mixture of 'eq'
> and regexp tests, which would be less clear.
In terms of just the condition, I think
if ($_ eq 'foo' or $_ eq 'bar' or /\Afred:(foo|baz)\z/) {
...
}
is actually clearer. On the other hand, the equivalence tests do make the real code more verbose:
if ($_ eq 'foo' or $_ eq 'bar') {
say "got $_";
}
elsif (/\Afred:(foo|baz)\z/) {
say "got $1";
}
else {
die "malformed line: $_";
}
which may be considered to be more complicated. The equivalent
if (/\A(?:fred:)?(foo|baz)\z/) {
say "got $1";
}
else {
die "malformed line: $_";
}
shouldn't trip it.
But the simple case you originally gave is really what the policy is targeting. And if you've got a large set of potential cases, you should really be using a hash lookup, rather than a set of equivalence tests.
In your more complicated example, the larger scope is what makes the code simpler in your approach. But this policy doesn't look at the larger scope, it simply looks at the regex. Yes, this is a limitation, but, given that, the policy is doing precisely what it is supposed to do.
RT for rt.cpan.org