Matt  Ward
Matt Ward

Reputation: 85

Why is this Perl foreach loop only executing only once?

I am trying to copy the content of three separate .vect files into one. I want to do this for all 5,000 files in the $fromdir directory.

When I run this program it generates just a single modified .vect file in the output directory. If I include the close(DATA) calls after individual while loops inside the foreach loop, I get the same behavior: a single output file in the output directory instead of the wanted 5,000 files.

I have done some reading, and at first thought I may not be opening the files. But if I print($vectfile) in the foreach loop every file name in the directory is printed.

My second thought was that it was how I was closing the files, but I get the same behavior whether I close the file handles inside or outside the foreach loop.

My final thought was maybe I don't have write permission to the file or directory, but I don't know how to change this.

How can I get this loop to run all 5,000 times and not just once?

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

my $dir = "D:\\Downloads";

# And M3.1 and P3.1
my $subfolder = "A0.1";

my $fromdir = $dir . "\\" . $subfolder;

my @files = <$fromdir/*vect>;

# Top of file
my $readfiletop = "C:\\Users\\Owner\\Documents\\MoreKnotVis\\ScriptsForAdditionalDataSets\\VectFileHeader.vect";

# Bottom of file
my $readfilebottom = "C:\\Users\\Owner\\Documents\\MoreKnotVis\\ScriptsForAdditionalDataSets\\VectFileCloser.vect";


foreach my $vectfile ( @files ) {

    say("$vectfile");

    my $count        = 0;
    my $readfilebody = $vectfile;
    my $out_file     = "D:\\Downloads\\ColorsA0.1\\" . "$count" . ".vect";

    $count++;

    # open top part of each file
    open(DATA1, "<", $readfiletop) or die "Can't open '$readfiletop': $!";

    # open bottom part of each file
    open(DATA3, "<",  $readfilebottom) or die "Can't open '$readfilebottom': $!";

    # open a file to read
    open(DATA2, "<", $vectfile) or die "Can't open '$vectfile': $!";

    # open a file to write to
    open(DATA4, ">" ,$out_file) or die "Can't open '$out_file': $!";

    # Copy data from VectFileTop file to another.
    while ( <DATA1> ) {
        print DATA4 $_; 
    }

    # Copy the data from VectFileBody to another.
    while ( <DATA2> ) {
        print DATA4 $_, $_ if 8..12;
    }


    # Copy the data from VectFileBottom to another.
    while ( <DATA3> ) {
        print DATA4 $_;
    }    
}

close( DATA1 );
close( DATA2 );
close( DATA3 );
close( DATA4 );

print("quit\n");

Upvotes: 1

Views: 387

Answers (2)

Borodin
Borodin

Reputation: 126772

You seem to be clinging to a specific form of code in fear of everything falling apart if you change a single thing. I recommend that you dare to stray a little more from the formula so that the code is more concise and readable

The problem is that you reset your $count to zero before processing each input file, so all the output files have the same name and overwrite one another. The remaining output file contains only the data from the last input file

Here's a refactoring of your code. I can't guarantee that it will run correctly but it looks right and does compile

  • I've added use autodie to avoid having to check the status of every IO operation

  • I've used the same lexical file handle $fh for all the input file. Opening another file on a file handle that is already open will close it first, and a lexical file handle will be closed by perl when it goes out of scope at the end of the block

  • I've used a while loop to iterate over the input file names instead of reading the whole list into an array which unnecessarily uses an additional variable @files and wastes space

  • I've used forward slashes instead of backslashes in all the file paths. This is fine in library calls on Windows: it is only a problem if they appear in command line input

I hope you'll agree that this form is more readable. I think you would have stood a much better chance of finding the problem if your code were in this form

use strict;
use warnings;
use autodie;
use feature qw/ say /;

my $indir     = 'D:/Downloads';
my $subdir    = 'A0.1';                    # And M3.1 and P3.1
my $extrasdir = 'C:/Users/Owner/Documents/MoreKnotVis/ScriptsForAdditionalDataSets';

my $outdir     = "$indir/Colors$subdir";
my $topfile    = "$extrasdir/VectFileHeader.vect";
my $bottomfile = "$extrasdir/VectFileCloser.vect";

my $filenum;

while ( my $vectfile = glob "$indir/$subdir/*.vect" ) {

    say qq/Processing "$vectfile"/;

    $filenum++;

    open my $outfh, '>', "$outdir/$filenum.vect";

    my $fh;

    open $fh, '<', $topfile;
    print { $outfh } $_ while <$fh>;

    open $fh, '<', $vectfile;
    while ( <$fh> ) {
        print { $outfh } $_, $_ if 8..12;
    }

    open $fh, '<', $bottomfile;
    print { $outfh } $_ while <$fh>;
}

say 'DONE';

Upvotes: 3

Valdi_Bo
Valdi_Bo

Reputation: 31011

You construct the output file name including $count in it.

But note what you do with this variable:

  • initially, but inside the loop you set it to 0,
  • the output file name is constructed with 0 in it,
  • then you increment it, but this has no effect, because this variable is again set to 0 in the next execution of the loop..

The effect is that:

  • the loop executes the required numer of times,
  • but the output file name every time contains 0 as the "number", so you keep overwriting the same file with a new content.

Move my $count = 0; instruction before the loop and everything should be OK.

Upvotes: 3

Related Questions