Using foreach instead of map and grep in perl

Question

my @a_columns = map { s/^"|"$|\n|\r|\n\r|"//g; $_ } split /;/, $s_act_line;

above mentioned is my code. i am getting automatic warning while merging this code because of the usage of the map function. please help me to convert this using for or foreach or any loop.

I have already tried couple of ways but nothing works for me.

please help me guys

Answer 1

The warning appears to follow Perl::Critic (even though it seems to be issued by your IDE).

The variable $_ in map, but also in grep and foreach, is an alias for the currently processed array element. So once it gets changed the input array gets changed! This is usually unwanted, and can surely be considered a tricky practice in general, thus the warning.

But in this case the input for map is the list produced by split, used on your variable. So code in map's block cannot change your variables by changing $_. Then it is safe to tell Perl::Ciritic to ignore this statement

my @a_columns = 
    map { s/^"|"$|\n|\r|\n\r|"//g; $_ } split /;/, $s_act_line;  ## no critic

or for more than one statement

## no critic

... code that Perl::Critic should ignore

## use critic

If the warning is indeed produced by Perl::Critic (and not by IDE) this should stop it.

Better yet, the idiom used in map's block is unneeded with Perl versions from 5.14, since we got the non-destructive substitution. With it s///r returns the changed string, leaving the original unchanged. So you can do

my @a_columns = map { s/^"|"$|\n|\r|\n\r|"//gr } split /;/, $s_act_line;

which is also much cleaner and safer. Now Perl::Critic should not flag this.

Answer 2

The problem is that you're modifying $_ in the map, which means you're modifying the values passed to map. That can lead to surprises. Switching to for wouldn't help.

It's actually harmless in this case since split returns temporary values, but the following are "clean" alternatives:

my @a_columns =
   map { ( my $tmp = $_ ) =~ s/^"|"$|\n|\r|\n\r|"//g; $tmp }
      split /;/, $s_act_line;

use List::MoreUtils qw( apply );

my @a_columns =
   apply { s/^"|"$|\n|\r|\n\r|"//g; }
      split /;/, $s_act_line;

my @a_columns =
   map { s/^"|"$|\n|\r|\n\r|"//rg }
      split /;/, $s_act_line;