--- 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]
