On Sunday 13 September 2009 02:43:52 Jenda Krynicky wrote: > From: Shlomi Fish <shlomif@iglu.org.il> > > > > use FileHandle; > > > my $signature = new FileHandle; > > > $signature->open("<$signature_file")or die "Could not open > > > file\n"; > > > > You should use IO::Handle instead of File::Handle (or IO::File in your > > case), and use the three args open. It's nice you've used die. > > Nope. You should not be bothering with thatever class is at the > moment behind the Perl filehandles and just use open() as a function. > Not everything has to be an object > > open my $SIGNATURE, '<', $signature_file > or die "Could not open '$signature_file': $^E\n"; > Indeed. No need to call "$SIGNATURE" with all-caps. "$signature_fh" is good enough. I should note that if you do "use IO::Handle;" at the top, then you can all methods on $signature_fh. > > $signature is not a good name for a filehandle - better call it > > $signature_fh. > > I use all capitals for filehandles. Both oldstyle and lexical. I dislike all-capitals because it's like shouting and are harder to read. But this is a colour of the bike-shed argument: http://bikeshed.com/ > > > > my $fileHandle = $_[0]; > > > > Accessing $_[$idx] is not very robust - you should do: > > > > <<< > > my $fileHandle = shift; > > > > > > Or: > > > > <<< > > my ($fileHandle) = @_; > > Nope. Accessing $_[$idx] all over the place within the subroutine > would be prone to errors and inconvenient (though sometimes > necessary), what syntax do you use to copy the parameters into > lexical variables is largely unimportant. And sometimes you can't > just shift() off parameters from @_, you need to keep them there. > Generally, I sometimes use something like: sub my_method { my $self = shift; my ($param1, $param2, $param3) = @_; . . . if (COND()) { return $self->other_method(@_); } } Normally I don't like using $_[0], $_[1], etc. directly. The only use I can think for them is to modify them in place using aliases. But in this case it is better and safer to pass them as references. > > > my $customer_msg_html = $customer_msgStart_html . OrderFromForm_html() > > > . $customer_msgEnd_html . $scalar_sig; > > > > Your style is inconsistent between camelCase, and > > underscore_separated_identifiers. > > Maybe there is a meaning to the underscores on some places and case > change in others. Don't be so quick. The fact that you do not see a > pattern from a very short example doesn't necessarily mean there > isn't one. It may very well be too big to fit in the small cut hole > that you see. > Maybe. It's still hard to read. Regards, Shlomi Fish > Jenda > ===== Jenda@Krynicky.cz === http://Jenda.Krynicky.cz ===== > When it comes to wine, women and song, wizards are allowed > to get drunk and croon as much as they like. > -- Terry Pratchett in Sourcery > -- ----------------------------------------------------------------- Shlomi Fish http://www.shlomifish.org/ Freecell Solver - http://fc-solve.berlios.de/ Chuck Norris read the entire English Wikipedia in 24 hours. Twice.Thread Previous | Thread Next