Your code seems to be the equivalent of this code:
{
my %seen;
local($^I, @ARGV) = ("", $outputfile);
while (<>) {
next if $seen{$_}++;
print;
}
}
It's a good thing to learn about in-place editing mode.
| [reply]
[d/l] |
This is very good advice, in principle, especially for the benefit of other readers, but for the OP evidence is that won't listen in any case. Indeed I'm afraid to have to do an ad hominem attack -and I shamelessly admit I'm doing one!-, but I doubt he's really willing to learn, let alone to listen. Which makes one wonder why he keeps asking, instead! Mistery...
| [reply] |
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?
| [reply]
[d/l] |
Failing to check the return value of open() is pretty illogical. | [reply] |
Where does the function "remove" come from? It isn't a built-in function. Do you mean "unlink"?
The parameters for "rename" should be the other way around and do not need quotes around them.
Why declare "our @data" and not "my @data"?
If the file is huge, the process will be slow since you're slurping the whole file in. It would be better to read and process line by line.
-imran | [reply] |
The file names are the correct way round. I want the content of the old file name to be the same as the content of the new file name.
| [reply] |
