in reply to Code review
First, I don't think
is doing what you think it is. Ponder the difference betweenwhile (<>) { chomp; s/ //g; tr/a-z/A-Z/; } if (/^GRD/) {
andwhile ( ... ) { ... } if ( ... ) { ... }
Next, can you explain what @GRD = split /,"?|""?/; is supposed to be doing? It leads me to believe that there's more to your data than the sample shows. Is your data a CSV file with quoted values?while ( ... ) { ... if ( ... ) { .... } }
And what is close ARGV if eof; supposed to be doing? Is there a file handle named ARGV that you opened previously? If so, that's the not the right way to test for eof.
Your field routine looks like it was written by someone who speaks C, but who hasn't yet come to terms with Perl, either in terms of control structures or in terms of how to deal with variable names. If I'm reading your intent correctly, it can be collapsed into something like
There are are better ways of doing this, but this gives you an example to chew on that illustrates some of the aspects of Perl that C programmers can gain great advantage in learning.my @GRD = split /,/; warn "line $.: wrong number of attributes\n" unless @GRD == 6; check($GRD[0], 1, @GRDF1); check($GRD[1], 2, @GRDF2); ... check($GRD[5], 6, @GRDF6); sub check ( my($GRD, $col, $@GRDF) = @_; return if grep { $GRD eq $_ } @GRDF; print "line $.: \"$GRD\" is invalid in column $col\n"; }
|
|---|
| Replies are listed 'Best First'. | |
|---|---|
|
Re: Code review
by Abigail-II (Bishop) on Sep 18, 2003 at 09:27 UTC | |
|
Re: Re: Code review
by mhearse (Chaplain) on Sep 18, 2003 at 04:41 UTC |