For Programmers: Free Programming Magazines  


Home > Archive > PERL Beginners > May 2007 > Script is skipping my If statement that checks for a regular file









You are viewing an archived Text-only version of the thread. To view this thread in it's original format and/or if you want to reply to this thread please [click here]

 

Author Script is skipping my If statement that checks for a regular file
sintral@gmail.com

2007-05-24, 9:58 pm

The entire code is below. The goal is to accept a single file as an
argument and then print each line in the file preceded by a 3 digit
line number (only accepts 999 lines).
For example: perl thissscript.pl example.txt

001 This is the first line
002 so on and so forth

-------------------------------------------------------------------------------------

#!/usr/bin/perl -w

unless (@ARGV=1) # Verify that only one argument was given
{
print "You must enter exactly 1 argument",
"\n";
exit 1;
}

open(FILE,"$ARGV[0]"); # Use the open command to create a file
handler

if (-f "$ARGV[0]") # Verify that the command line argument is a file
{
$COUNTER=1; # Start the counter at 1

while (<FILE> )
{
if ($COUNTER<10)
{
print "000";
print "$COUNTER: $_";
}
if (($COUNTER>9) && ($COUNTER<100))
{
print "00";
print "$COUNTER: $_";
}
if (($COUNTER>99) && ($COUNTER<1000))
{
print "0";
print "$COUNTER: $_";
}

$COUNTER++; # Increment the counter

}
}
else
{
print "The argument was not a regular file",
"\n";
exit 1;
}
exit 0;

Paul Lalli

2007-05-24, 9:58 pm

On May 24, 10:16 am, sint...@gmail.com wrote:
> The entire code is below. The goal is to accept a single file as an
> argument and then print each line in the file preceded by a 3 digit
> line number (only accepts 999 lines).
> For example: perl thissscript.pl example.txt
>
> 001 This is the first line
> 002 so on and so forth
>
> -------------------------------------------------------------------------=

--=AD----------
>
> #!/usr/bin/perl -w
>
> unless (@ARGV=3D1) # Verify that only one argument was given


No, that's not what that does. That assigns @ARGV to be the list
containing (1). You meant =3D=3D, not =3D.


> {
> print "You must enter exactly 1 argument",
> "\n";
> exit 1;
>
> }
>
> open(FILE,"$ARGV[0]"); # Use the open command to create a file
> handler


* Useless use of double quotes
* using global bareword filehandles
* using two-argument form of open
* not checking for errors.

That last one is the worst mistake. You have NO WAY of verifying that
this open succeeded, because you didn't ask Perl to tell you if it
didn't.

open my $FILE, '<', $ARGV[0] or die "Cannot open $ARGV[0]: $!";

> if (-f "$ARGV[0]") # Verify that the command line argument is a file


Another useless use of double quotes. Please see `perldoc -q
quoting`. More importantly, though, don't you think this check should
happen BEFORE you try to open the file?


> {
> $COUNTER=3D1; # Start the counter at 1
>
> while (<FILE> )
> {
> if ($COUNTER<10)
> {
> print "000";
> print "$COUNTER: $_";
> }
> if (($COUNTER>9) && ($COUNTER<100))
> {
> print "00";
> print "$COUNTER: $_";
> }
> if (($COUNTER>99) && ($COUNTER<1000))
> {
> print "0";
> print "$COUNTER: $_";
> }


Gah!!! Get rid of all of these if statements, and replace them with
this one line:
printf "%03d: $_", $COUNTER;

Paul Lalli

nobull67@gmail.com

2007-05-24, 9:58 pm

On May 24, 3:41 pm, Paul Lalli <mri...@gmail.com> wrote:
> On May 24, 10:16 am, sint...@gmail.com wrote:
> open my $FILE, '<', $ARGV[0] or die "Cannot open $ARGV[0]: $!";
>
>
> Another useless use of double quotes. Please see `perldoc -q
> quoting`. More importantly, though, don't you think this check should
> happen BEFORE you try to open the file?


Actually that would create a race. If you really want to ensure that
the thing you've opened is a regular file then you should apply the -f
to the filehandle not the filename.

But then again why do you care? Why should users full-feature OSs of
be prevented from using pipes?

Paul Lalli

2007-05-24, 9:58 pm

On May 24, 12:35 pm, "nobul...@gmail.com" <nobul...@gmail.com> wrote:
> On May 24, 3:41 pm, Paul Lalli <mri...@gmail.com> wrote:
>
>
>
>
> Actually that would create a race. If you really want to ensure that
> the thing you've opened is a regular file then you should apply the -f
> to the filehandle not the filename.


Fair point. But can we at least agree that testing to see if the
argument is a file *after* the attempted open makes the least
sense? ;-)

Paul Lalli

sintral@gmail.com

2007-05-24, 9:58 pm

On 24 May, 13:23, Paul Lalli <mri...@gmail.com> wrote:
> On May 24, 12:35 pm, "nobul...@gmail.com" <nobul...@gmail.com> wrote:
>
>
>
>
>
>
> Fair point. But can we at least agree that testing to see if the
> argument is a file *after* the attempted open makes the least
> sense? ;-)
>
> Paul Lalli


Actually, it's a two part error that I receive now:

Name "main::FILE" used only once: possible typo at ./linenum line 25.
readline() on unopened filehandle FILE at ./linenum line 25.

unless (@ARGV==1) # Verify that only one argument was given
{
print "You must enter exactly 1 argument",
"\n";
exit 1;
}

if (-f $ARGV[0]) # Verify that the command line argument is a file
{
# Use the open command to create a file handler
open my $FILE, '<', $ARGV[0] or die "Cannot open $ARGV[0]: $!";

$COUNTER=1; # Start the counter at 1

while (<FILE> )
{
printf "%03d: $_", $COUNTER;

$COUNTER++; # Increment the counter

}
}
else
{
print "The argument was not a regular file",
"\n";
exit 1;
}
exit 0;

Steven M. O'Neill

2007-05-24, 9:58 pm

<sintral@gmail.com> wrote:
>Actually, it's a two part error that I receive now:
>
>Name "main::FILE" used only once: possible typo at ./linenum line 25.
>readline() on unopened filehandle FILE at ./linenum line 25.


Too bad the script's not working, because then you could have
used it to show us which line is the 25th.

>unless (@ARGV==1) # Verify that only one argument was given
>{
> print "You must enter exactly 1 argument",
> "\n";
> exit 1;
>}
>
>if (-f $ARGV[0]) # Verify that the command line argument is a file
>{
> # Use the open command to create a file handler
> open my $FILE, '<', $ARGV[0] or die "Cannot open $ARGV[0]: $!";

^^^^^ what is the value of this scalar variable?
>
> $COUNTER=1; # Start the counter at 1
>
> while (<FILE> )

[...]
--
Steven O'Neill steveo@panix.com
Brooklyn, NY http://www.panix.com/~steveo
sintral@gmail.com

2007-05-24, 9:58 pm

On 24 May, 13:40, ste...@panix.com (Steven M. O'Neill) wrote:
> <sint...@gmail.com> wrote:
>
>
> Too bad the script's not working, because then you could have
> used it to show us which line is the 25th.
>
>
>
> ^^^^^ what is the value of this scalar variable?
>
>
>
> [...]
> --
> Steven O'Neill ste...@panix.com
> Brooklyn, NY http://www.panix.com/~steveo


You know, you're right. That was stupid on my part.
Line 25 reads

while (<FILE> )

sintral@gmail.com

2007-05-24, 9:58 pm

On 24 May, 13:43, sint...@gmail.com wrote:
> On 24 May, 13:40, ste...@panix.com (Steven M. O'Neill) wrote:
>
>
>
>
>
>
>
>
>
>
>
>
> You know, you're right. That was stupid on my part.
> Line 25 reads
>
> while (<FILE> )


I've removed some unnecessary lines and spaces from the code, here is
a cleaned up version with some manual line numbers:

1 unless (@ARGV==1) # Confirm that only one argument was given
2 {
3 print "You must enter exactly 1 argument", "\n";
4 exit 1;
5 }
6
7 if (-f $ARGV[0]) # Verify that the command line argument is a file
8 {
9 open my $FILE, '<', $ARGV[0] or die "Cannot open $ARGV[0]: $!";
# Create a file handler
10
11 $COUNTER=1; # Start the counter at 1
12
13 while (<FILE> )
14 {
15 printf "%03d: $_", $COUNTER; # Print the line number, followed
by the line
16 $COUNTER++; # Increment the counter
17 }
18 }
19 else
20 {
21 print "The argument was not a regular file", "\n";
22 exit 1;
23 }
24 exit 0;

Paul Lalli

2007-05-24, 9:58 pm

On May 24, 1:32 pm, sint...@gmail.com wrote:

> Actually, it's a two part error that I receive now:
>
> Name "main::FILE" used only once: possible typo at ./linenum line 25.
> readline() on unopened filehandle FILE at ./linenum line 25.
>
> open my $FILE, '<', $ARGV[0] or die "Cannot open $ARGV[0]: $!";
> while (<FILE> )


I assumed that when I made the change to your open() statement, you
would be able to make the corresponding change to your readline
operator. My mistake.

while (<$FILE> )

Paul Lalli

sintral@gmail.com

2007-05-24, 9:58 pm

On 24 May, 13:54, Paul Lalli <mri...@gmail.com> wrote:
> On May 24, 1:32 pm, sint...@gmail.com wrote:
>
>
>
>
> I assumed that when I made the change to your open() statement, you
> would be able to make the corresponding change to your readline
> operator. My mistake.
>
> while (<$FILE> )
>
> Paul Lalli


And that was the problem, thanks Paul for gerberring me through that
and all your helpful advice.

Sponsored Links







Also available: Server administration forum archive | Web Design forum archive | Software forum archive | Hardware reviews archive

Copyright 2008 codecomments.com