|Perl: the Markov chain saw|
Re: On being 'critical'by pemungkah (Priest)
|on Dec 14, 2006 at 19:41 UTC||Need Help??|
The solutions already posted are useful and accurate, but maybe it would be useful to go into the why a bit.
(As an aside: picking up a copy of Perl Best Practices is a great idea, because it explains all this stuff in detail. However, let me summarize a bit here.)
First: the lexical filehandle. These guys give you two advantages: one, they're lexicals instead of globals, so it's impossible for someone else to open or close your filehandle (unless you hand it to them, of course). When the lexical goes out of scope, the filehandle will get closed for you.
In your case, this is a very important consideration: if your code opens the filehandle and then uses it in multiple calls to the subs in TagPrint, you'd need the our variable, or to store a reference to the filehandle scalar in a TagPrint object; that way it won't go out of scope until either program end (the our variable) or the object is destroyed.
Second: three-argument open separates the mode in which you're opening the file from the file itself. The potential problem we're avoiding is this one:
This opens $file for input. However, if $file starts with a >, it's opened for output (and clobbered). Three-argument open fixes this, because the second argument is "how do I open this", and the third is "just the filename". Filenames with a leading > now can't screw you up. This isn't really a consideration for you, as you're duping a filehandle, and are using an explicit string.
Final thoughts: Perl::Critic is not the end source of all Perl wisdom, and TheDamian is not Moses, bringing the Perl Commandments from on high: you've actually chosen a pretty good way of keeping random people from accidentally opening or closing your filehandle by pushing it out of the global namespace. Perl::Critic recommends things because they're ways of making your code more understandable and less likely to have bugs, not because they're the Only Right Way.