loan tran wrote: > Please critize my scripts and suggest better ways to > accomplish the same task. >
You asked for it. > (I know it's a good practice to put use strict in Perl > script but I'm struggling to make my scripts work with > use strict.The script below work fine if I comment out > use strict.) > Turn on use strict. Turn on use strict. Turn on use warnings. Turn on use strict. > [EMAIL PROTECTED] /home/sybase/scripts/pl => > test_load_lcmr.pl 275 2005 06 > Global symbol "$isql" requires explicit package name > at ././test_load_lcmr.pl line 54. > Global symbol "$loader" requires explicit package name > at ././test_load_lcmr.pl line 66. > Global symbol "$loaderpswd" requires explicit package > name at ././test_load_lcmr.pl line 66. > Global symbol "$isql" requires explicit package name > at ././test_load_lcmr.pl line 78. > Execution of ././test_load_lcmr.pl aborted due to > compilation errors. > You are getting these errors because you have not declared all of your variables. A great place to start: http://perl.plover.com/FAQs/Namespaces.html > > [EMAIL PROTECTED] /home/sybase/scripts/pl => test_load_lcmr.pl 275 2005 06 > Global symbol "$isql" requires explicit package name at ././test_load_lcmr.pl > line 54. > Global symbol "$loader" requires explicit package name at > ././test_load_lcmr.pl line 66. > Global symbol "$loaderpswd" requires explicit package name at > ././test_load_lcmr.pl line 66. > Global symbol "$isql" requires explicit package name at ././test_load_lcmr.pl > line 78. > Execution of ././test_load_lcmr.pl aborted due to compilation errors. > > #----------------- script name: test_load_lcmr.pl -------------------------- > #!/usr/bin/perl > # Purpose: bcp lcmrldr ftp file provided by CPMC into lcmrdb..ldr_stage and > exec sp to load lcmr > #use strict; > use warnings; > require "/home/sybase/scripts/pl/global.pl"; You might consider making the above dynamic or relative. And 'use' is often favored to 'require' these days. perldoc -f use perldoc lib perldoc FindBin > our $recipients = '[EMAIL PROTECTED]'; > our $dockdir ="/ftpsite/as400/glmgr"; > #our $isql = "/opt/finance/sybase/OCS-12_0/bin/isql -U$loader -P$loaderpswd > -w133 "; #now get from global.pl > our $bcp = '/opt/finance/sybase/OCS-12_0/bin/bcp'; > our $company = <$ARGV[0]>; > our $year = <$ARGV[1]>; > our $period = <$ARGV[2]>; You might try using Getopt::Long for option processing. I have a sample template available at: http://danconia.org/cgi-bin/request?handler=Content;content=StaticPage;label=getopt_long_template > our $server ='THEBRAIN'; # for testing > our $debug =0; > our $load_file = 'cpmc_lcmr_'.$year.'_'.$period.'.'.'csv'; > our $log_file = '/home/sybase/scripts/logs/load_lcmr.pl.log'; > qx (cp /dev/null $log_file ); You shouldn't shell out for the above process which appears to just empty a file. You can use Perl builtins for that, and should. When copying a file you should consider using File::Copy. > #print "\nserver = $server; company = $company; Year = $year; period = > $period \n"; > qx(echo "server = $server; company = $company; Year = $year; period = $period > \n" >>$log_file); > There is no reason to shell out to echo, print should work fine. Why the commented out line above? If you are trying to write to the log file, you should open it for printing. perldoc perlopentut perldoc -f open perldoc -f print perldoc -f close > ##### Main Part ############## > > if($#ARGV != 2 or $company !~ /^\d{3}$/ or $year !~ /^\d{4}$/ or $period !~ > /^\d{2}$/){ > print "\nIncorrect arguments. See usage below:\n"; > &Usage; Using & is the older method, the above is better written: usage(); But see my template on a nice way to handle option processing, usage statements, and documentation all rolled into one. > } > else{ > unless(-f "$dockdir/$load_file"){die qq(Source file "$dockdir/$load_file" > not found. Script aborted.\n); } > &deleteBeforeLoad($company,$year,$period); Again, no '&'. > &bcp_in; Same, the above should take its arguments and optionally return value(s). You are using globals in your subroutines which is evil, strict will help, as will moving your subs into a library. > if ($debug ==0){&run_lcm2_post($company,$year,$period)} #end if; > ¬ify_mail; > } #end else > > ###### Sub Routines ############### > > sub Usage{ > print "\n********* USAGE ***********\n"; > print "\n$0 Company Year Period\n"; > print "Company: 3 digits required (275)\n"; > print "Year : 4 digits required (2005)\n"; > print "Period : 2 digits required (06)\n"; > print "\n**************************\n"; > exit; > } #end sub Usage > > sub deleteBeforeLoad{ > print "---Delete staging table before bcp in \n"; > qx($isql -S$server<<EOF>> $log_file You might consider using the DBI module and its drivers depending on what database this is. > select getdate() > go > exec lcmrDB..delete_LDR_Stage $_[0],$_[1],$_[2] > go > EOF > ); > &check_error; > } #end sub delete > > sub bcp_in{ > print "---Start bcp in \n"; > qx($bcp lcmrDB..LDR_Stage in $dockdir/$load_file -c -U$loader -S$server > -P$loaderpswd -t',' >>$log_file ); Again, dispense with the shelling out and move any commands that can be handled in Perl into the native code. Especially simple things like 'wc', 'cut', 'echo', etc. [snip] The rest of the script follows the same issues as above. Good luck, http://danconia.org -- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] <http://learn.perl.org/> <http://learn.perl.org/first-response>
