On Monday 12 July 2004 08:21, jason corbett wrote:
>
> Here is the script that I am using. Remember that the script creates
> the file perfectly and stores the file with data in a folder on the
> server. The emailed copy is being sent out blank.
>
> #!/usr/bin/perl
use warnings;
> use strict;
> use DBI;
> use lib '/home/samcsm/jason/myperl/lib/perl5/site_perl/';
> use MIME::Lite;
>
>
> our ($dbh); #
> our (@files); #
> our ($filename); #
> our ($errorlog); #logs all successes and failures of the report
> our ($dirfolder); #where the treatment report is sent prior to emailing
> our (@record); #holds all records returned from query
> our ($success_var); #flags the script if completion was successful or not
> our ($sql); #
> our ($sth); #
> our ($dayreport); #
> our ($process_date); #used in the log file to tell what date and time a file
> complete or failed
> our ($month); #
> our ($day); #
> our ($year); #
> our ($truename); #Time stamps the name of each treatment report AFTER the GENERIC
> name is changed
> our (@times); #Helps Timestamp the treatment reports each day
> our ($reportlog); #Folder for keeping up with all Treatment reports
> our ($fourdayreport); #the path that leads to the location of the generic 4 day
> report
> our ($recordlist); #allows the for manipulation of data retrieved from query
You should really use 'my' instead or 'our' to declare variables
unless you REALLY REALLY need 'our'. The parentheses are not
required unless you are declaring more then one variable.
our $variable; # no parens required
our ( $var1, $var2 ); # Must use parens
If you had used the warnings pragma you would have been told that
@files, $filename and @record are not being used anywhere which
brings up the next point. You should declare your variables when
you first use them instead of all at the beginning of the program.
> #set up for the file naming covention
> @times = (localtime)[3..5]; # grabs year/month/day values
^^^^ ^^^^^^^^^^^^^^
I had to do a double take when I read this. The comment says
year/month/day but the code says day/month/year.
> $month = $times[1] + 1;
> $day = $times[0];
> $year = $times[2] + 1900;
You don't really need the @times array at all:
my ( $day, $month, $year ) = (localtime)[3..5];
$month += 1;
$year += 1900;
> $truename = "Tr0"."$month"."0"."$day"."$year"."_4day.csv\n"; #Time stamps each file
> once generic file is renamed
Wow, interpolation AND concatenation. Perhaps either:
my $truename = 'Tr0' . $month . 0 . $day . $year . "_4day.csv\n";
Or:
my $truename = "Tr0${month}0$day${year}_4day.csv\n";
> [snip code]
>
> dateme(); #call dateme function and store value in $process_date
Your comment says that you are storing the value in $process_date
so why don't you code it like that:
my $process_date = dateme();
> $success_var = data_collect(); #call data_collection function and store the word
> "success" #or "failed" in $success_var variable
>
> my $logvariable = logresults($success_var); #call the logresults function and send
> the "success" or "failed"
>
> mailman(); #call mailman function
>
> print "$logvariable\n"; #print the results returned from logresults function
>
> close(OUTFILE); #close file for errorlog and query
> close(ERRORLOG);
>
> $dbh->disconnect; #disconnet
>
>
> #-----------------------SUB ROUTINE FOR COLLECTING
> DATA-----------------------------------------------------------------------
[ whitespace changed to show error ]
> sub data_collect
> {
> unless (open(OUTFILE,">$fourdayreport")) #open 4daytreatment.csv
> {
> die open(ERRORLOG, ">>$errorlog") && #or die and open errorlog.txt
> print ERRORLOG "Sorry file $dayreport couldn't be created\n";
> return "Failed";
> } #print into errorlog.txt message
> else{
> while( my @record= $sth->fetchrow_array()) #while theres data, collect
> {
> $recordlist=join(",",@record); #separate records with a "," and
> store
> print OUTFILE "$recordlist\n"; #print to newformatted records to
> #the generic file 4daytreatment.csv
> }
> return "success"; #send back the word "success" when done
> }
You are missing the closing subroutine brace!!!
> [snip code]
>
> #-----------------------SUB ROUTINE for Time
> Stamp-------------------------------------------------------------------------
> sub dateme{
> $process_date=localtime(); #call localtime funcion and store as scalar value
>
> }return $process_date; #return scalar value of the localtime function
Your return statement is OUTSIDE the subroutine!!!
You are returning a value but you DON'T USE IT!!!
You can use the scalar() function to return a scalar value:
sub dateme {
return scalar localtime;
}
John
--
use Perl;
program
fulfillment
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
<http://learn.perl.org/> <http://learn.perl.org/first-response>
