On Thursday 08 July 2004 12:49, FyD wrote:
>
> Hi,

Hello,

> Here is a short perl script:
>
>    open (FILE, "<$START");

You should *ALWAYS* verify that the file opened correctly:

open FILE, "<$START" or die "Cannot open $START: $!";


>    $nbc=0;

You should use lexically scoped variables instead of package variables.

my $nbc = 0;


>    foreach(<FILE>){

Reading a filehandle in a foreach loop means that the whole file has to 
be converted to a list before the loop starts.  It is more efficient to 
use a while loop instead.

while ( <FILE> ) {


>       if (/^XXX/ig)   { $atmnb[$nbc]++;}
>       if (/^YYY/ig)   { $atmnb1=$atmnb[0];}
>       if (/^ZZZ/ig)   { $nbc++;}

The /g option is used to find multiple matches in the string (globally) 
but you have anchored the match at the beginning of the string so it 
can only match once at the beginning of the string.  But even if it 
wasn't anchored, you are using the match as an if condition so matching 
multiple times is superfluous as one match is just as true as two or 
more matches.

    if ( /^XXX/i ) { $atmnb[ $nbc ]++ }
    if ( /^YYY/i ) { $atmnb1 = $atmnb[ 0 ] }
    if ( /^ZZZ/i ) { $nbc++ }

Of course you could do that without using regular expressions.

    if ( 'XXX' eq uc substr $_, 0, 3 ) { $atmnb[ $nbc ]++ }
    if ( 'YYY' eq uc substr $_, 0, 3 ) { $atmnb1 = $atmnb[ 0 ] }
    if ( 'ZZZ' eq uc substr $_, 0, 3 ) { $nbc++ }


>    }
>    if ($atmnb1 eq "") { $atmnb1=$atmnb[0];}

It could be that the value of $atmnb1 is undef.  You have to use the 
defined function to test for this.

$atmnb1 = $atmnb[ 0 ] if not defined $atmnb1 or not length $atmnb1;

But this should also work (unless the value in $atmnb1 is 0):

$atmnb1 ||= $atmnb[ 0 ];


>    close(FILE);
>    $nbc++;


John
-- 
use Perl;
program
fulfillment


-- 
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
<http://learn.perl.org/> <http://learn.perl.org/first-response>


Reply via email to