banker123 <> wrote:
> Question
> 1. How do I sort the variables in a scalar context not an array?
That question makes no sense.
"scalar" means "a single thing".
You cannot sort a single thing, you need more than one thing
before you can impost an order (sorting) on the things.
> Should I be reading the variables I have extracted into an array?
Probably.
> 2. This program executes every 15 seconds to output the contents of
> the directory. The output is used to control work flow and meet crucial
> deadlines. Is this effeciently written?
"premature optimization is the root of all evil"
(google it)
> Any suggestions?
Don't concern yourself much with efficiency until _after_ it has
been demonstrated that what you already have is too slow.
Is it taking close to 15 seconds to execute?
If not, then you are done without having to spend any time tweeking stuff.
> My first "production" perl program, after reading and learning perl for
> about a month.
>
> #Open directory
> my $dir="G:/Formware/files/";
I would suggest leaving the trailing slash out of there.
It looks confusing when you use it later.
> opendir DH, $dir or die "Cannot open$!";
It would be nice to know the name of the directory that you were
attempting to open, so you should include that info (along with
some delimiters) in your diagnostic message:
opendir DH, $dir or die "Cannot open '$dir' $!";
> #Get system date in the format mmddyy
> @months = qw(01 02 03 04 05 06 07 08 09 10 11 12);
You should always put these:
use warnings;
use strict;
at the top of every Perl program. They will catch many common
mistakes for you.
> @days = qw(01 02 03 04 05 06 07 08 09 10..31);
You never use the @days array in your subsequent code, so why is it there?
> ($second, $minute, $hour, $dayOfMonth, $month, $yearOffset, $dayOfWeek,
> $dayOfYear, $daylightSavings) = localtime();
If you use a "Slice" (see that section in perldata.pod) you can get
just the ones you want without a bunch of dummy variables that you
never use:
my($day, $month, $year) = (localtime)[3,4,5];
> $dayOfMonth=sprintf("%02d", $dayOfMonth);
$day = sprintf '%02d', $day;
$month = sprintf '%02d', $month + 1;
> $date = "$months[$month]$dayOfMonth06";
^^
^^
Your program will break in about two months...
$year = sprintf '%02d', ($year + 1900) % 100;
my $date = "$month$day$year";
> #Read each file beginning with system date from directory
> @files=grep(/^$date/,readdir(DH));
Whitespace is not a scarce resource. Feel free to use as much of it
as you like to make your code easier to read and understand:
@files = grep(/^$date/, readdir(DH));
Even better, eliminate unnecessary parenthesis:
@files = grep /^$date/, readdir DH;
or
@files = grep { /^$date/ } readdir DH;
> closedir (DH);
>
>
> foreach $file (@files) {
Here you read things into an array when you do not need to. Below
you do not read things into an array when you do need to. Get
the logic right first, then worry about performance.
foreach my $file ( grep /^$date/, readdir DH ) { # no @files temp array
> open (data, "$dir$file") or die "Cannot open file: $!";
Your working program will become a non-working program when you update
your perl and it includes a new function named data().
It looks like you are missing a directory separator, but you're not.
It would be better if it didn't look like a mistake. Leave the
trailing slash out of $dir, and put it in explicitly when it is needed.
You should report the filename in the diagnostic message again.
The 3-argument form of open() is much much better, so you should get
used to using that form all of the time.
open my $data, '<', "$dir/$file" or die "Cannot open '$dir/$file' $!";
> while ( <data> ) {
while ( <$data> ) {
> if ( /JOBNAME/ ) {
> $job = substr($_,8,
;
> chomp($job);
Why do you give substr args that capture a newline only to remove the newline?
Just grab one less character, then you won't need a chomp:
my $job = substr($_, 8, 7);
> }
> elsif ( /\.TIF\s{12}/ ) {
> my $box = substr($_,73,6);
> printf "$box $job $file\n";
> last;
> }
You should line up your closing brace with the keyword that goes with
the opening brace, and indent the code to show its structure:
elsif ( /\.TIF\s{12}/ ) {
my $box = substr($_,73,6);
printf "$box $job $file\n";
last;
}
--
Tad McClellan SGML consulting
Perl programming
Fort Worth, Texas