Sorting files into directories based on contents

[PerlTwerp]

I'm sure it will be obvious by my post, but my knowledge of Perl, and programming for that matter, is limited so please be gentle.
I am trying to read in a directory of files, sort the files into other directories based on their contents
(one particular field actually) and then have the file not exist in the original directory.

Here is the code I came up with but it seems a little barbaric and like it could be better, there are some environment
variables that I don't think will affect the purpose. Please let me know if any other details are needed.


use strict;
use File::Copy;
use Cwd;

#---------Split
my $directory = "$ENV{FTP_PRODUCTION}\\CDSEXP\\GOL";
my @filenames = glob ("$directory\\*.tpe");

my $start1 = '0';
my $len1 = '8';
my $start2 = '128';
my $len2 = '5';
my $newloc1 = $ENV{FTP_PRODUCTION} . '\CDSEXP\GOL\refund_proceed';
my $newloc2 = $ENV{FTP_PRODUCTION} . '\CDSEXP\GOL\other';
my $newloc3 = $ENV{FTP_PRODUCTION} . '\CDSEXP\GOL';

foreach my $filename (@filenames){

open (FILEIN, $filename) or die "Unable to open in file.";

while (defined(my $record = <FILEIN>)) {

my $rec = substr($record, $start1, $len1);
my $type = substr($record, $start2, $len2);
if ($rec eq "10025900"){
if ($type eq "MANDV"){
close FILEIN;
move ("$filename", "$newloc1") or die "Unable to refund_proceed";
}else{
close FILEIN;
move ("$filename", "$newloc2") or die "Unable to other";
}
}else{
}
}
}#End of the 4 each file

 

[Ghodmode]

Hi PerlTwerp,
I think that the only real way to do what you're trying to do is to open the file as you're already doing.
The only real problem I saw was that your code has the potential to try to read another line from the tpe file after closing it. That's easily corrected with a [tt]next[/tt]. Here's my version of your code. The only real difference is that there's a [tt]next[/tt] after closing the file so that you don't try to read another line.

####################
## PRAGMA
####################
use strict;
use warnings;
use File::Copy;
use Cwd;

####################
## VARIABLES
####################
#---------Split
my $directory = "$ENV{FTP_PRODUCTION}\\CDSEXP\\GOL";
my @filenames = glob( "$directory\\*.tpe" );

my $start1  = '0';
my $len1    = '8';
my $start2  = '128';
my $len2    = '5';
my $newloc1 =  $ENV{FTP_PRODUCTION} . '\CDSEXP\GOL\refund_proceed';
my $newloc2 =  $ENV{FTP_PRODUCTION} . '\CDSEXP\GOL\other';
my $newloc3 =  $ENV{FTP_PRODUCTION} . '\CDSEXP\GOL';

####################
## MAIN
####################
for ( @filenames ) {
	my $filename = $_;
	open ( FILEIN, $filename ) or die "Unable to open in file.";

	while ( <FILEIN> ) {
		my $record = $_;
		my $rec    = substr( $record, $start1, $len1 );
		my $type   = substr( $record, $start2, $len2 );

		if ( $rec eq "10025900" ) {
			my $destination;
			if ( $type eq "MANDV" ) {
				$destination = $newloc1
			} else {
				$destination = $newloc2;
			}

			close FILEIN;
			move( $filename, $destination ) or 
				die "Unable to move $filename to $destination";
			next;
		}
	}
}

--
-- Ghodmode

Give a man a fish and he'll come back to buy more... Teach a man to fish and you're out of business.

 

[travs69]

I'm not sure the next is needed.. he closes the filehandle so there will nothing else to read.

Personal Opinion Warning
I like this:
my $destination = $type eq "MANDV" ? $newloc1 : $newloc2;

instead of this:
if ( $type eq "MANDV" ) {
$destination = $newloc1
} else {
$destination = $newloc2;
}

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Travis - Those Who Say It Cannot Be Done Are Usually Interrupted by Someone Else Doing It; Give the wrong symptoms, get the wrong solutions;

 

[KevinADC]

actually the code is pretty good. The biggest fault is the "die " functions do not include "$!" in the output and you did not compile the script with the warnings pragma:

use warnings;

Any improvements to your code will be minimal because there is no major problems. You should document the code with comments for future reference. You would be surprised how quickly you forget why you coded something the way you did.

------------------------------------------
- Kevin, perl coder unexceptional!

 

[Ghodmode]

Travis,
You're right that the [tt]next[/tt] is not strictly needed. However, it's a good idea. If you use the warnings pragma... and you should always use the warnings pragma (ref: Perl Best Practices), you'll see a message telling you so:

readline() on closed filehandle FILE at ./test.pl line 9.

I also prefer the C-style conditional operator (ref: perlop.perl.org) you used, but it's not as easy to understand when trying to explain something.

--
-- Ghodmode

Give a man a fish and he'll come back to buy more... Teach a man to fish and you're out of business.

 

[KevinADC]

I don't think that is correct but I could be wrong. The "while" statement checks the condition first before doing anything. This has the affect of terminating a loop when 'eof' is reached when reading from a <filehandle>.

------------------------------------------
- Kevin, perl coder unexceptional!

 

[PerlTwerp]

Cool, thanks for the input!! Having more meaningful output for the die to produce makes a lot of sense.