On Jun 13, 2013, at 10:57 PM, lee wrote:

> Hi,
> 
> so I've done this script now --- my fourth perl script ever --- and
> since I'm getting so much help here, I thought I'd post it here.  I'm
> sure it could be done much better, it's just plain and simple.
> 
> It has one problem: The list of closed files can grow indefinitely, and
> since the script checks whether a file that has been closed is already
> listed, performance will degrade with the number of files on the closed
> list increasing.  This check isn't exactly needed and can be removed;
> other than that, I don't know (yet) what to do about that.

Some comments on your code:

1. Rather than doing this:

  while ( <$curridx>) {
    chomp $_;
    my $current_file = $_;

you should do this:

  while ( my $current_file = <$curridx> ) {
    chomp($current_file);

2. I would return undef from gethash() if the file doesn't exist, rather than 
'~'. Undef is false in a logical test, and can also be tested with the define() 
function.

3. You are reading the close.list file for each file tested. It would be more 
efficient to read the close.list file and put the closed file names into a 
hash, and then you can test each file to see if it is in that hash, either with 
the exists() function, or just be testing the hash value. A hash evaluates to 
undef (false) if the key does not exist in the hash.

4. You can use the join(EXPR,LIST) function here:

  print $file $key . ":" . $size . ":" . $mtime . ":" . $hash . "\n";

and do this instead:

    print $file join(':',($key,$size,$mtime,$hash)), "\n";



--
To unsubscribe, e-mail: beginners-unsubscr...@perl.org
For additional commands, e-mail: beginners-h...@perl.org
http://learn.perl.org/


Reply via email to