Update: I didn't see demerphq's Read more tag. This doesn't add much if anything to what he said. Mea culpa.
I don't have time to comment on the whole thing, but thought a comment on the following piece of code might be useful to you:
my $filename = "";
$filename = $hash{"LIS_FILE"} if($node eq "sam");
$filename = $hash{"LIS_FILE_TEST"} if($node eq "cad2");
chomp $filename;
my $htmfilename = "";
$htmfilename = $hash{"HTM_FILE"} if($node eq "sam");
$htmfilename = $hash{"HTM_FILE_TEST"} if($node eq "cad2");
chomp $htmfilename;
my $sqt_path = "";
$sqt_path = $hash{"SQT_PATH"} if($node eq "sam");
$sqt_path = $hash{"SQT_PATH_TEST"} if($node eq "cad2");
chomp $sqt_path;
my $database_name = "";
$database_name = $hash{"DB_NAME"} if($node eq "sam");
$database_name = $hash{"DB_NAME_TEST"} if($node eq "cad2");
chop $database_name;
my $mail_server_name = "";
$mail_server_name = $hash{"MAIL_SERVER"} if($node eq "sam");
$mail_server_name = $hash{"MAIL_SERVER_TEST"} if($node eq "cad2");
chomp $mail_server_name;
First, Where did
%hash come from? I am assuming that it is a by product of the
read_config($node); call. It would be more obvious if you commented this, or even better ( IMO ) made
read_config($node); return a hash-ref.
Second, it seems that you have take a liking to post_conditionals. Each of these pairs could be rewritten as if {} else {} constructs. With the dual post-conditionals you are forcing the evaluation of both conditions every time, potentially doubling your work.
Third, there is a pattern to what you are doing in this block -- refactor it. One suggestion is:
my $postfix = '';
if ( $node eq 'cad2' ) {
$postfix = '_TEST';
}
my $attrlist = { filename =>
$hash{"LIS_FILE${postfix}"},
htmfilename =>
$hash{"HTM_FILE${postfix}"},
sqt_path =>
$hash{"SQT_PATH${postfix}"},
database_name =>
$hash{"DB_NAME${postfix}"},
mail_server_name =>
$hash{"MAIL_SERVER${postfix}"}
};
Now obviously your information is stored in a hash and will change the way you refer to the data -- but you could set it up so that you have scalars:
my ($filename, $htmfilename,
$sqt_path, $database_name,
$mail_server_name) = ('', '', '', '', '');
my @refs = (\$filename, \$htmfilename,
\$sqt_path, \$database_name,
\$mail_server_name);
foreach my $attr ( qw( LIS_FILE HTM_FILE SQT_PATH
DB_NAME MAIL_SERVER ) )
{
my $store = shift @refs;
$$store = $hash{"${attr}${postfix}"};
}
Of course this method will lead to a coupling between the
@refs and list of fields to be gathered.
As well, this code does not test whether the desired
%hash entry exists. Including that you might get:
my ($filename, $htmfilename,
$sqt_path, $database_name,
$mail_server_name) = ('', '', '', '', '');
my @refs = (\$filename, \$htmfilename,
\$sqt_path, \$database_name,
\$mail_server_name);
foreach my $attr ( qw( LIS_FILE HTM_FILE SQT_PATH
DB_NAME MAIL_SERVER ) )
{
my $store = shift @refs;
if ( defined $hash{"${attr}${postfix}"} ) {
$$store = $hash{"${attr}${postfix}"};
} else {
die "Missing required config data: ${attr}${postfix}";
}
}