There's more than one way to do things | |
PerlMonks |
Critique of some perl code.by jwkrahn (Abbot) |
on Apr 17, 2022 at 07:03 UTC ( [id://11143017]=perlmeditation: print w/replies, xml ) | Need Help?? |
Hello Everyone, I occasionally like to download perl programs off the web to see how they work. And if there are any bugs I try to report back to the original author. This code was found verbatim (except for the file name) in two different files.
It looks like there is a race condition between the file test and the file open. The file test does not print a message if the file is not found but the file open does. If the file cannot be opened a message is printed and then another message is printed when readline tries to read from an invalid file handle. The file name is copied to a string.
Data is assigned to $_ and then copied to $line but $_ is never used inside the loop.
This is the line that gobsmacked me. I ran some tests with Benchmark and re 'debug' compared to the FAQ answer "s/\s+$//;" and it turns out that this is a good example of what not to do. This substitution was found in five places in one file and two places in the other file.
Not as bad as the previous line but still not as good as the FAQ answer.
Correct code, nothing to see here.
Why check for whitespace after you've just removed it?
This code is where the work is done and obviates the need for the previous five lines. split(" ", $line) removes all whitespace. foreach will not loop if @words is empty. $word =~ m/^#/ will skip to the next line if a '#' comment is found.
Correct code, nothing to see here.
The conditional is not really required. Where I found this code: <Reveal this spoiler or all in this thread>
Back to
Meditations
|
|