in reply to Style Comments

Just a few suggestions - I only have a few minutes... A different take on the specific problems in your code, rather than the Tie::File approach - hopefully you'll get some good tips. open FH, "< $indir/$file" or printf "can't open %s\n",$file; This printf will go to STDOUT (insert favourite OS's equivalent here), you really want this to go to STDERR instead, and nicely formatted etc...try
open FH, "< $indir/$file" or warn "can't open $file :: $!" and return;
warn goes to STDERR and $! has the OS specific message about why the open failed - we then return to the caller for the next file.

$line = <FH>; $_ = $line; close FH; $field1 = m/^Clock/; # Check for C +lock at beginning of file printf "\n$file"; if ( $field1 == "1"){
there is no need to assign to $_, and unless you really have no choice you should consider attempts to as a sign you dont understand the operator about to work in $_ by default. this bit is better rewritten as
$line = <FH>; close FH; printf "\n$file"; if ( $line =~ m/^Clock/){


Next, you should take the repeated opening and closing of the same file as a clue that things could be better done - if you need to go to the start again, look at seek(), if you need the current offset look at tell(). I/O is a relatively heavyweight operation, and your process will (on a *nix system) probably get swapped out while this happens. The more you can defer I/O the better - I like to use Tie::Handle, which ties my output to an array, but make all my code look like I am doing I/O. I have experienced up to two orders of magnitude improvement in perform from this simple approach .

Getting the last line of the file - try File::ReadBackwards, or, if you know the data well enough, grab the last n bytes of the file via operations like seek and read, and scan for the last line separator (dont just look for '\n', use $/.)

hard-coded paths - try for better independence from OS spcifics - try using File::Spec to build up your paths - if portability is important to you (it may not be now, but later...)

passing and returning arrays - try not to pass or return an array - Perl ends up making copies and this is inefficient. Use references instead -
sub ret_a_ref { my @arr; ... return \@arr; } my $a_ref = ret_a_ref(); my $first = $a_ref->[0]; ...

Replies are listed 'Best First'.
Re: Re: Style Comments
by mildside (Friar) on Sep 05, 2003 at 05:06 UTC
    The more you can defer I/O the better - I like to use Tie::Handle, which ties my output to an array, but make all my code look like I am doing I/O. I have experienced up to two orders of magnitude improvement in perform from this simple approach .
    Wow, what a great tip. Two orders of magnitude! I'll try that next chance I get.

    Cheers!