I prefer to lay out code like this myself.
if ($foo) {
$foo--;
&stuff;
}
if ($bar) {
$baz += $foo
} else {
$baz = $foo < 9
? 3
: 1;
}
There is less vertical whitespace (by putting the opening brace on the same
lines as the if/else statements. It is also clearer to see which 'if' the
'else' is refering to by placing it on the same line as the closing brace.
Just my opinion, no doubt there are hundreds more. I agree with the rest of
your mail though. Whitespace in code can greatly improve readability if used
correctly, and consistently throughout your programs. Once you find a
readable coding style, it's good practice to stick to it as it will make
reading and debugging your own code much easier.
http://www.perlmonks.org/index.pl?node=perlman%3Aperlstyle&lastnode_id=148
John
-----Original Message-----
From: Curtis Poe [mailto:[EMAIL PROTECTED]]
Sent: 10 January 2002 16:45
To: Scott; [EMAIL PROTECTED]
Subject: Re: Manageable code
--- Scott <[EMAIL PROTECTED]> wrote:
> I admit I am a newbie and making this harder than it really is :) But, I
> have a program that is growing by the minute and I want to split it apart
> into chunks or sub routines.
>
> Here is an example:
>
> sub init_type_99{
> print NEWQUOTES "99"; #RecordType
> printf NEWQUOTES ("%08d", "1"); #FileBatchTotal
> printf NEWQUOTES ("%08d", $count-1); #TotalRecords
> printf NEWQUOTES ("%-237s"); #RecordFiller
> print NEWQUOTES "\n";
> }
>
> Yet when I call the sub doing this:
>
> init_type_99();
> I do not get the results I was looking for. Is it a bad idea to do a sub
> like this and reserve it to only return a value?
Scott,
A subroutine doesn't necessarily need to return a value, but there are a
couple of things you
could do, in my opinion, to improve it.
1. Fix the indentation.
Good indentation does not mean good code, but bad indentation is usually
indicative of bad code
and it's harder to read. Consider the difference between these:
if ($foo){$foo--;&stuff;};if($bar){$baz += $foo}
else {$baz = $foo < 9 ? 3 : 1;};
Ugh. That's pretty confusing. What is the 'else' associated with? It's
easy to get this wrong.
Consider an alternative:
if ($foo)
{
$foo--;
&stuff;
}
if ($bar)
{
$baz += $foo
}
else
{
$baz = $foo < 9
? 3
: 1;
}
You might object to all of the whitespace in this version, but it's much
easier to read and to
determine the scope of everything.
2. Subs should usually be black boxes. Stuff goes in and stuff goes out.
You should be able to
cut and paste a subroutine directly into another piece of code without any
problems. This means
that a subroutine should be dependant on its arguments and nothing else.
sub foo
{
my $bar = shift;
if ( $bar > $baz )
{
return;
}
return ++$bar;
}
The above snippet is problematic because it depends upon $baz being declared
outside of itself.
Two immediate problems crop up. The first is that changing the name $baz
means you need to track
it down in the subs that use it and change it there, too (part of the goal
of programming should
be to minimize bugs stemming from excessive maintenance). The other problem
is that this sub is
not general purpose. What if you want a different test? You can't. Here's
a better way:
sub foo
{
my ( $bar, $baz ) = @_;
if ( $bar > $baz )
{
return;
}
return ++$bar;
}
You can now change the variable names to your heart's content and you have a
more general
subroutine! Good stuff. Let's apply these thoughts to your code:
sub init_type_99
{
my ( $fh, $count ) = @_;
my $record = "9900000001"; # recordtype and
filebatchtotal
$record .= sprintf("%08d", $count-1); # total records
$record .= ' ' x 237, "\n"; # filler
print $fh $record;
}
Now, you might not like exactly how I applied everything above (I'd probably
build the record
differently), but I think it gives you a better idea of what I am referring
to. Since this looks
like a routine to print the header of a report, I might generalize it more.
Maybe passing in the
record type, file batch total and filler length, for example, to make this
applicable to different
record types.
Incidentally, to pass a filehandle in a variable (as I did in the example),
check out the
FileHandle module or IO::File.
Cheers,
Curtis "Ovid" Poe
=====
"Ovid" on http://www.perlmonks.org/
Someone asked me how to count to 10 in Perl:
push@A,$_ for reverse q.e...q.n.;for(@A){$_=unpack(q|c|,$_);@a=split//;
shift@a;shift@a if $a[$[]eq$[;$_=join q||,@a};print $_,$/for reverse @A
__________________________________________________
Do You Yahoo!?
Send your FREE holiday greetings online!
http://greetings.yahoo.com
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
--------------------------Confidentiality--------------------------.
This E-mail is confidential. It should not be read, copied, disclosed or
used by any person other than the intended recipient. Unauthorised use,
disclosure or copying by whatever medium is strictly prohibited and may be
unlawful. If you have received this E-mail in error please contact the
sender immediately and delete the E-mail from your system.
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
