If you're using Perl 5.005_03 or higher, this is how I'd write your code:
open my $results, '<', $outputfile
or die "Cannot open '$outputfile' for reading: $!\n';
open my $cleaned, '>', $new_outputfile
or die "Cannot open '$new_outputfile' for writing: $!\n";
{
my %seen;
local $_;
while ( <$results> ) {
chomp;
next if $seen{ $_ }++;
print $cleaned $_, "\n";
}
}
# Always close in the reverse order of opening.
close $cleaned;
close $results;
unlink $results
or die "Cannot unlink '$outputfile': $!\n";
File::Copy::move( $new_outputfile, $outputfile )
or die "Cannot rename '$new_outputfile' to '$outputfile': $!\n";
The changes:
- Use 3-arg open for safety.
- Use lexicals for filehandles instead of globals. (That's the difference between 'my $fh' and 'FH'.)
- Always check the return values of system commands.
- If you fail a system command, then stop processing immediately. It's dangerous to continue unless you have a way of cleaning up what went wrong.
- Don't read the file into memory. It might be small now, but it will probably grow.
- If you intend on using $_, localize it so that you don't trample on anyone.
- You're not paying for your whitespace, so use whitespace liberally. It will increase readability in the long run.
- If you open it, then close it as soon as you're done with it.
- If you open two things, close them in reverse order.
My criteria for good software:
- Does it work?
- Can someone else come in, make a change, and be reasonably certain no bugs were introduced?