I have the following script in Perl that does the following:
Prompts user for time in epoch. If user enters '########', the query will look for all reports generated after that date/time. If user does not input time, it will just look for all reports in past day.
Connects to a database, runs a query to receive reports
I create a directory of the current date (i.e.
Jun25
)I iterate through every row in the results and I save the contents of the xml file into an actual file and name the file
[customer name]-[epoch time].xml
I am trying to clean up my code and have it run more efficiently.
#!/usr/bin/perl
#use strict;
use DBI;
use File::Slurp;
use Encode;
use utf8;
use POSIX qw(strftime);
use File::Slurp;
use XML::XPath;
use XML::XPath::XMLParser;
$| = 1;
print "Please enter epoch time of which to receive reports or leave blank to receive last day of reports: ";
my $epoch = <STDIN>;
chomp($epoch);
if ($epoch =~ /\D/) {$epoch = time() - 86400}
###Start Database
my $dsn = join "", (
"dbi:ODBC:",
"Driver={SQL Server};",
"Server=SERVER;",
"Database=DATABASE",
);
my $user = q/USERNAME/;
my $password = q/PASSWORD/;
my $db_options = {
PrintError => 1,
RaiseError => 1,
#LongTruncOk => 1,
LongReadLen => 100000000,
};
print "CONNECTING TO DATABASE...";
# Connect to the data source and get a handle for that connection.
my $dbh = DBI->connect($dsn, $user, $password, $db_options)
or die "Can't connect to $dsn: $DBI::errstr";
print "CONNECTED\nFETCHING ROWS AND PARSING...\n";
my $sql = "SELECT [receivedTime], [reportXml] from UsageReport WHERE [receivedTime] > " . $epoch . " AND [reportXml] NOT LIKE '%exclude%'";
# Prepare the statement.
my $sth = $dbh->prepare($sql)
or die "Can't prepare statement: $DBI::errstr";
$sth->execute();
###End Database
#Make Directory of Current Date
$datestring = strftime "%b%d", localtime;
mkdir $datestring;
chdir $datestring;
# Fetch and display the result set value.
my @matrix;
my $count = 0;
while (my @row = $sth->fetchrow_array ) {
my ($time, $report) = @row; #time, report
$count++;
eval {
my $xp = XML::XPath->new(xml => $report);
my $customer = $xp->getNodeText('/report/instances/instanceroot/instanceheader/customer');
if ($customer eq "") {next;}
our $ext = $customer . "-" . $time . ".xml";
write_file($ext, $report);
};
if ( $@ ) {printf "ERROR ";}
else {printf "$count ";}
}
# Disconnect the database from the database handle.
$dbh->disconnect;
print "\nCOMPLETED REPORT PARSING\nLatest Report Date: ";
2 Answers 2
The program asks to input the time first, before trying to connect to the database. If the database connection fails, the user might be frustrated for having entered something only to watch the program crash. It would be better to move the input part right before it's really needed.
There are some unused elements:
use File::Slurp;
appears twice, delete the second@matrix
is never used
The comment at the end of this line is pointless:
my ($time, $report) = @row; #time, report
You should use DBI prepared statements to avoid injecting a variable into your SQL statement, as this variable content is directly controlled by the user of your program.
warnings
and uncommentingstrict
may be a good idea. Also you can run profiler if you''re concerned with performance. You're checking return value for->prepare
but not for->execute
(when connectingRaiseError => 1
should do these checks for you).printf
andour
doesn't make sense here. \$\endgroup\$