4

I am using regex in perl to convert string to hex, however, when I do this, I get a warning from either perl critic or perl:

#$test is defined, i just put the regex code snippet here...
#this will trigger a warning from perl critic
#warning: use of regex expression without "/x" expression..
$text =~  s/(.)/sprintf("%x",ord($1))/eg;

#this will trigger a a warning at run time 
#warning: "uninitialized value $1 in regexp compilation"
$text =~   m{s/(.)/sprintf("%x",ord($1))/eg}x;

Is there a way to write the above code that doesn't get a warning or feedback from Perl critic?

I think the issue is because ord is handling the undefined values, and when you put in /x then checking of the regex expression thinks that the value of $1 is invalid .

Bostwick
  • 696
  • 1
  • 12
  • 24

1 Answers1

8

This critic is what is called a false positive. There's no need or reason for /x here. If you try to silence every critic, you'll end up with weird code. That said, the critic was recommending

s/(.)/sprintf("%x",ord($1))/exg

Furthermore, it probably makes no sense to avoid converting newlines. If so, you want to use /s too.

s/(.)/sprintf("%x",ord($1))/sexg
ikegami
  • 367,544
  • 15
  • 269
  • 518
  • Yeah, I don't know why you wrapped the whole thing in m{...}x.. that completely changed the meaning. – Mark Reed Oct 17 '12 at 01:32
  • Can you point me to where I can read more about what the /x is doing here (/what the parameters are there) ? [link]http://search.cpan.org/dist/Perl-Critic/lib/Perl/Critic/Policy/RegularExpressions/RequireExtendedFormatting.pm is why i put the m{}x . In any case putting the /x clears the perl critic error and runs error free(so I don't think this was a false positive) ! – Bostwick Oct 17 '12 at 13:47
  • 1
    `/x` allows you to include whitespace and comments in the pattern. See [perlre](http://perldoc.perl.org/perlre.html). The critic is trying to promote making your regex readable. The example shows how to add `/x` to `m//`, but you're not using `m//`. You're suppose to be adding `/x`, not switching to `m//`. – ikegami Oct 17 '12 at 20:40