sudharsan501 has asked for the wisdom of the Perl Monks concerning the following question:

Below is the script is created to find a missing file in two directories and copy it to the directory i specify.. It works 100% good..but i just wanted to know if i have put the script in the right way.. Thanks!!
use File::Copy; use Date::Manip; my $search_in="U:\\Temp"; my $search_local="D:\\my"; my $copy_to=$ARGV[1]; $db_name=$db_date=$ARGV[0]; #$db_name=$db_date=$_; $db_name =~ s/\d|\.trn|\_backup_//g; $db_date =~s/[^\d]//g; $miss_file_date = UnixDate(DateCalc(ParseDate($db_date), "-10 minutes" +), "%Q%H%M"); $missing_file=$db_name."_backup_".$miss_file_date.".trn"; print "\n\nThe database name is: $db_name\n"; print "Error occured in file[input file] :$ARGV[0] \n"; print "File missing for date: $miss_file_date\n"; print "\nThe missing file is: $missing_file\n\n"; print "Searching..please wait\n"; foreach( glob("$search_local/$missing_file") ){ if (-e $_) { #$copy_to="F:\\Backup\\$db_name\\CURR" copy ($_,$copy_to) or print "Can't copy file to $curr_folder\n" and di +e; print "\nFile found!!\n"; print "File found in $search_local\n"; print "\nThe missing file $missing_file is copied to $copy_to\n"; } else{ print "local search \n"; print "Unable to find the missing file $_\n"; print "Search in sql\n"; goto search2; } } search2: foreach( glob("$search_in/$missing_file") ){ if (-e $_) { #$copy_to="F:\\Backup\\$db_name\\CURR" copy ($_,$copy_to) or print "Can't copy file to $curr_folder\n" and di +e; print "\nFile found!!\n"; print "File found in $search_in\n"; print "\nThe missing file $missing_file is copied to $copy_to\n"; } else{ print "Unable to find the missing file $_\n"; } }

Replies are listed 'Best First'.
Re: Search for a missing file..iss my script correct?
by roboticus (Chancellor) on Aug 06, 2010 at 05:10 UTC

    sudharsan501:

    Here are some suggestions:

    • You should get into the habit of using strict and warnings. They'll help you avoid many coding errors.
    • You should indent your code to make it easier to see the structure. Using a good programming editor will help you adjust your indents as you need them. You can also use perltidy (I've never used it, but it's frequently suggested).
    • You don't need to use a goto statement in your program. You're just trying to exit the loop, so use the last statement, which will do exactly that. You then don't need the label search2:.
    • You use extremely similar code in two locations. That's a tipoff that a subroutine is probably a good idea.
    • You're printing some error messages at the front of your program for no apparent reason. You should actually check to see whether the file is missing before proclaiming that it is missing.
    • There are other things, but this will get you started.

    I've put most of my suggestions into your code, shown below. Note: I didn't run it--tuning it up is left as an exercise for the reader...

    use strict; use warnings; use File::Copy; use Date::Manip; my $search_in="U:\\Temp"; my $search_local="D:\\my"; my $copy_to=$ARGV[1]; $db_name=$db_date=$ARGV[0]; $db_name =~ s/\d|\.trn|\_backup_//g; $db_date =~s/[^\d]//g; $miss_file_date = UnixDate(DateCalc(ParseDate($db_date), "-10 minutes" +), "%Q%H%M"); $missing_file=$db_name."_backup_".$miss_file_date.".trn"; print "\n\nThe database name is: $db_name\n"; print "Error occured in file[input file] :$ARGV[0] \n"; print "File missing for date: $miss_file_date\n"; print "\nThe missing file is: $missing_file\n\n"; print "Searching..please wait\n"; perform_search($search_local, "local"); perform_search($search_in, ""); sub perform_search { my $search_dir = shift; my $local_flag = shift; foreach ( glob("$search_dir/$missing_file") ){ if (-e $_) { copy ($_,$copy_to) or print "Can't copy file to $curr_fold +er\n" and die; print "\nFile found!!\n"; print "File found in $search_in\n"; print "\nThe missing file $missing_file is copied to $copy +_to\n"; } else{ print "local search\n" if $local_flag eq "local"; print "Unable to find the missing file $_\n"; if ($local_flag eq "local") { print "Search in sql\n"; last; } } } }

    You'll notice that I used a flag to let the subroutine work in two different ways, depending on whether it was local or not. I don't know if the last statement is really necessary or not, so I left it in.

    ...roboticus

Re: Search for a missing file..iss my script correct?
by jwkrahn (Abbot) on Aug 06, 2010 at 05:12 UTC
    $db_date =~s/[^\d]//g;

    That is usually written as:

    $db_date =~ s/\D//g;

    Or better as:

    $db_date =~ s/\D+//g;

    Or even better as:

    $db_date =~ tr/0-9//cd;

    foreach( glob("$search_local/$missing_file") ){ if (-e $_)

    It looks like you are looking for a single file so there is no need for the glob and foreach loop, just the -e test should be enough.


    goto search2;

    That is ususally written as:

    last;

    But since you don't really need the loop that is moot.