CODEWITHSUNDEEP
perl
GrSrv
GrSrv

Reputation: 559

foreach and special variable $_ not behaving as expected

I'm learning Perl and wrote a small script to open perl files and remove the comments

# Will remove this comment

my $name = ""; # Will not remove this comment

#!/usr/bin/perl -w <- wont remove this special comment

The name of files to be edited are passed as arguments via terminal 

die "You need to a give atleast one file-name as an arguement\n" unless (@ARGV);

foreach (@ARGV) {
    $^I = "";
    (-w && open FILE, $_) || die "Oops: $!";
    /^\s*#[^!]/ || print while(<>);
    close FILE;
    print "Done! Please see file: $_\n";
}

Now when I ran it via Terminal: perl removeComments file1.pl file2.pl file3.pl

I got the output: Done! Please see file:

This script is working EXACTLY as I'm expecting but

Issue 1 : Why $_ didn't print the name of the file?

Issue 2 : Since the loop runs for 3 times, why Done! Please see file: was printed only once?

How you would write this script in as few lines as possible?

Please comment on my code as well, if you have time.

Thank you.

Upvotes: 5

Views: 335

Answers (4)

Joni
Joni

Reputation: 111219

The while stores the lines read by the diamond operator <> into $_, so you're writing over the variable that stores the file name.

On the other hand, you open the file with open but don't actually use the handle to read; it uses the empty diamond operator instead. The empty diamond operator makes an implicit loop over files in @ARGV, removing file names as it goes, so the foreach runs only once.

To fix the second issue you could use while(<FILE>), or rewrite the loop to take advantage of the implicit loop in <> and write the entire program as:

$^I = "";
/^\s*#[^!]/ || print while(<>);

Upvotes: 9

David W.
David W.

Reputation: 107040

Simpler solution: Don't use $_.

When Perl was first written, it was conceived as a replacement for Awk and shell, and Perl heavily borrowed from that syntax. Perl also for readability created the special variable $_ which allowed you to use various commands without having to create variables:

while ( <INPUT> ) {
   next if /foo/;
   print OUTPUT;
}

The problem is that if everything is using $_, then everything will effact $_ in many unpleasant side effects.

Now, Perl is a much more sophisticated language, and has things like locally scoped variables (hint: You don't use local to create these variables -- that merely gives _package variables (aka global variables) a local value.)

Since you're learning Perl, you might as well learn Perl correctly. The problem is that there are too many books that are still based on Perl 3.x. Find a book or web page that incorporates modern practice.

In your program, $_ switches from the file name to the line in the file and back to the next file. It's what's confusing you. If you used named variables, you could distinguished between files and lines.

I've rewritten your program using more modern syntax, but your same logic:

use strict;
use warnings;
use autodie;
use feature qw(say);

if ( not $ARGV[0] ) {
    die "You need to give at least one file name as an argument\n";
}

for my $file ( @ARGV ) {
    # Remove suffix and copy file over
    if ( $file =~ /\..+?$/ ) {
       die qq(File "$file" doesn't have a suffix);
    }
    my ( $output_file = $file ) =~ s/\..+?$/./;  #Remove suffix for output
    open my $input_fh, "<", $file;
    open my $output_fh, ">", $output_file;
    while ( my $line = <$input_fh> ) {
       print {$output_fh} $line unless /^\s*#[^!]/;
    }
    close $input_fh;
    close $output_fh;
}

This is a bit more typing than your version of the program, but it's easier to see what's going on and maintain.

Upvotes: 0

user1558455
user1558455

Reputation:

Its not safe to use multiple loops and try to get the right $_. The while Loop is killing your $_. Try to give your files specific names inside that loop. You can do this with so:

foreach my $filename(@ARGV) {
    $^I = "";
    (-w && open my $FILE,'<', $filename) || die "Oops: $!";
    /^\s*#[^!]/ || print while(<$FILE>);
    close FILE;
    print "Done! Please see file: $filename\n";
}

or that way:

foreach (@ARGV) {
    my $filename = $_;
    $^I = "";
    (-w && open my $FILE,'<', $filename) || die "Oops: $!";
    /^\s*#[^!]/ || print while(<$FILE>);
    close FILE;
    print "Done! Please see file: $filename\n";
}

Please never use barewords for filehandles and do use a 3-argument open.

  • open my $FILE, '<', $filename — good

  • open FILE $filename — bad

Upvotes: 2

chrsblck
chrsblck

Reputation: 4088

Here's a more readable approach. 

#!/usr/bin/perl

# always!!
use warnings;
use strict;

use autodie;
use File::Copy;

# die with some usage message
die "usage: $0 [ files ]\n" if @ARGV < 1;


for my $filename (@ARGV) {
    # create tmp file name that we are going to write to
    my $new_filename = "$filename\.new";

    # open $filename for reading and $new_filename for writing
    open my $fh, "<", $filename;
    open my $new_fh, ">", $new_filename;

    # Iterate over each line in the original file: $filename,
    # if our regex matches, we bail out. Otherwise we print the line to
    # our temporary file.  
    while(my $line = <$fh>) {
       next if $line =~ /^\s*#[^!]/; 
       print $new_fh $line;
    }

    close $fh;
    close $new_fh;

    # use File::Copy's move function to rename our files. 
    move($filename, "$filename\.bak");
    move($new_filename, $filename);

    print "Done! Please see file: $filename\n";
}

Sample output:

$ ./test.pl a.pl b.pl 
Done! Please see file: a.pl
Done! Please see file: b.pl

$ cat a.pl
#!/usr/bin/perl

print "I don't do much\n"; # comments dont' belong here anyways

exit;

print "errrrrr";

$ cat a.pl.bak
#!/usr/bin/perl

# this doesn't do much
print "I don't do much\n"; # comments dont' belong here anyways

exit;

print "errrrrr";

Upvotes: 3

Related Questions