Thanks for the input.
My input data (which is coming out of a flat ascii file) looks similar to:
Jones, John           35 20 02 05/02/2001 F  060506050705040405047 11 04 01
Jones, John           35 20 02 05/09/2001 F  050604050705040405045 10 13 02
Jones, John           35 20 02 05/16/2001 F  050505040704040509048 09 01 03

How could I "pass this function an array, or format the string for a split() 
or something"?

Another thing I would really like to clean up is the redundancy in output...
is there a way I can write to both:
                printf OF (" %3u", @hole[$q]);
                printf OF2 (" %3u", @hole[$q]);
?????

Thanks again.

----Original Message Follows----
From: Aaron Craig <[EMAIL PROTECTED]>
To: [EMAIL PROTECTED]
Subject: Re: Re: Code Review
Date: Mon, 25 Jun 2001 11:47:34 +0200

At 15:42 22.06.2001 -0400, Brian Bukeavich wrote:
>Sorry, I just thought the text would be too long.
>
>=========================================================================
>#! /perl/bin/perl

first of all, you need to use strict!
<insert standard strict rant here>

this is going to cause your program to fail, because you haven't declared
your variables using my() or local().  However, it's very important that
you keep your namespace clean.

all your variables need to be declared thusly:

my $var = "value";

><snip>

>$n = 0;

for instance, if you now use $n in a subroutine (very likely that you'd
want to) and don't declare it with my inside the sub, you don't know if
you'll get the latest value of $n, the value that you want in the
subroutine, or what...dangerous!

><snip>


I bet you mean $prevname[0] here.

>         if (@prevname[0] ne "")
>                         {
>                         Prnt_Player_Stats ();
>                         }

a more Perl way to write this would be
Prnt_PlayerStats() if($prevname[0] ne "");

>sub Load_Stat_Rec ()
>         {
>         $fline0 = $_;
>         @name[$i]           = substr $fline0,0,22;
>         @course[$i]         = substr $fline0,22,2;
>         @tee[$i]            = substr $fline0,25,2;
>         @team[$i]           = substr $fline0,27,3;
>         @date[$i]           = substr $fline0,31,10;
>         @nine[$i]           = substr $fline0,42,1;
>         @score[$i]          = substr $fline0,45,18;           #046 - 063
>         @total[$i]          = substr $fline0,64,2;
>         @handicap[$i]       = substr $fline0,67,2;
>         @opponent[$i]       = substr $fline0,70,2;
>         @week[$i]           = substr $fline0,73,2;
>         @substitute[$i]     = substr $fline0,75,22;
>         @notes[$i]          = substr $fline0,98,25;

this is frightenenly complicated.  Couldn't you pass this function an
array, or format the string for a split() or something?

Check your array syntax.  I bet you want to use $notes[$i] here as well.

That's just for starters, I'm sure others will put their two bits in as 
well.

You definitely want to re-examine your program flow, as well -- or at least
your variable naming.  You may know that $i contains the index of the
current record, but if some poor schmuck has to maintain this code when
you're on vacation, they'll have to waste time figuring that out.  I'd call
it something like $lCurrentRecord.

I know it's very "C", but I really do like prefixes on my variable names
that tell me what kind of value I'm expecting the variable to hold, a
number, a string, an array, etc.  This is very useful when you have to
change or debug code that you wrote a year ago.

Example:
my $index = 1;
if fine, but what's wrong with saying
my $lIndex = 1;
6 months from now, I know that I'm expecting that variable to contain a
number, not a string.
more importantly:
sub ParseNames($)
         {
         my($raNames) = @_;
         ...
         }
instead of
sub ParseNames($)
         {
         my($names) = @_;
         ...
         }
6 months from now, when I'm looking at this code, I know I'm expecting this
subroutine to get passed an array reference, and nothing else.


Aaron Craig
Programming
iSoftitler.com


_________________________________________________________________
Get your FREE download of MSN Explorer at http://explorer.msn.com

Reply via email to