On Mon, Sep 30, 2002 at 02:37:52PM -0500, Lance Prais wrote:
[original code, slightly reformatted]
if ($partition eq "Public"){
open(PUBLIC_ALERTSFILE,">$public_alerts_index_txt_filename");
print PUBLIC_ALERTSFILE"$oid". "::" ."$title". "::" .."$partition". "::"
."$date\n";
close(PUBLIC_ALERTSFILE) || die "can't close $file: $!";
}
if ($partition eq any(qw/"Public" "OPSEC SDK")){
open(OPSEC_ALERTSFILE,">$opsec_alerts_index_txt_filename");
print OPSEC_ALERTSFILE"$oid". "::" ."$title". "::" .."$partition". "::" ."$date\n";
close(OPSEC_ALERTSFILE) || die "can't close $file: $!";
}
if ($partition eq any(qw/"Public" "OPSEC SDK" "Gold/Platinum")){
open(ADVANCED_ALERTSFILE,">$advanced_alerts_index_txt_filename");
print ADVANCED_ALERTSFILE"$oid". "::" ."$title". "::" .."$partition". "::"
."$date\n";
close(ADVANCED_ALERTSFILE) || die "can't close $file: $!";
}
if ($partition eq any(qw/"Public" "OPSEC SDK" "Gold/Platinum" "CSP")){
open(CSP_ALERTSFILE,">$csp_alerts_index_txt_filename");
print CSP_ALERTSFILE"$oid". "::" ."$title". "::" .."$partition". "::" ."$date\n";
close(CSP_ALERTSFILE) || die "can't close $file: $!";
}
[end original code]
Ok, for one, I don't think Janek intended you to actually use the
Quantum::Superpositions module in your production code. It was just one
method of doing it. $partition eq 'Public' || $partition eq 'OPSEC SDK' is
very likely faster, and doesn't require an external module to implement.
qw/"Public" "OPSEC SDK"/ will not give you what you intended. It results
in, effectively, the literal list ('"Public", '"OPSEC', 'SDK"'). See
perldoc -f qw, and the reference it points to.
Did you really intend for a line in every file to be added if $partition eq
'Public'? That's what happens with your code.
Each open in your code truncates the current file to 0 before appending new
text, and isn't checked for failure. I doubt you wanted to truncate the
file, and you should always check your open.
There are various methods for making your code much less redundant. The way
I would choose would probably go something like this:
my %partitions = (
'Public' => [$public_alerts_index_txt_filename,
$opsec_alerts_index_txt_filename,
$advanced_alerts_index_txt_filename,
$csp_alerts_index_txt_filename],
'OPSEC SDK' => [$opsec_alerts_index_txt_filename,
$advanced_alerts_index_txt_filename,
$csp_alerts_index_txt_filename],
'Gold/Platinum' => [$advanced_alerts_index_txt_filename,
$csp_alerts_index_txt_filename],
'CSP' => [$csp_alerts_index_txt_filename],
);
foreach my $filename (@{ $partitions{$partition} }) {
open(FILE, ">>$filename")
|| die("Unable to open file \"$filename\": \l$!.\n");
print FILE "$oid\::$title\::$partition\::$date\n";
close(FILE)
|| die("Unable to close file \"$filename\": \l$!.\n");
}
Even if you don't like that solution, you should compare it to your code and
see what optimizations, clarifications, correct code, etc. you can glean
from it. For example, the open mode causes the print to append to the file,
rather than overwrite it, and the open is checked.
Michael
--
Administrator www.shoebox.net
Programmer, System Administrator www.gallanttech.com
--
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
