Re^2: Unsuccessful stat on file test
by bluethundr (Pilgrim) on Mar 02, 2008 at 19:58 UTC
|
Thanks for the suggestion! I didn't even see the '\n' with my beginner's eyes! But now, my code reads like this: #!/usr/bin/perl
use warnings;
use strict;
sub filetest {
my @answer;
return "File does not exist\n";
unless -e $_;
push @answer, "readable "
if -r $_;
push @answer, "writable "
if -w $_;
push @answer, "executable "
if -x $_;
return @answer;
}
while (<>) {
s/\s*$//;
my @answer = &filetest($_);
print @answer;
}
And now, no matter what file I feed the program, it reports "File does not exist". Also, if I remove the line return "File does not exist\n";
unless -e $_;
the program executes quietly, not printing out any messages. I'm a little confused and befuddled by this.
| [reply] [d/l] [select] |
|
|
return "File does not exist\n";
unless -e $_;
needs to become
return "File does not exist\n"
unless -e $_;
the program works for me:
C:\Projekte>perl -w tmp.pl
tmp.pl
readable writable
So, maybe tell us more exactly what you're typing in, what files exist, and maybe consider adding some more debugging output to your file.
Your form of calling the filetest function is bad/misleading. filetest is not using any parameters passed to it, it uses the global $_. So you should either call it as filetest(); or make your subroutine actually use its argument.
As a style note, I never use the ampersand notation for function calls (&filetest()) - I find it too dangerous, because when I leave off the parentheses (&filetest;), that has the unintended side effect of calling filetest with the current values of @_ instead of passing no parameters at all. | [reply] [d/l] [select] |
|
|
Your code is almost there, but you have to remove the semicolon:
my @answer;
return "File does not exist\n"; # <- extra semicolon!
unless -e $_;
The next problem is: while (<>) {
If your program is named filetest.pl and you call it like:./filetest.pl *.txt
that loop is actually going to read each line of all the *.txt files in your current directory, and unless each line actually happens to be the name of a file in that directory, your program will correctly tell you that the file does not exists!
Try adding some extra output (what is $_ ?)
cheers | [reply] [d/l] [select] |
|
|
By gum! With your help (and the help of the other posters)...I think I've got it! Hard work do pay off! Well, the prog isn't quite finished. There's still some debugging messages in there, and some tidying up to do.
But basically it is now in a workable state. If I feed the program (which is named ex1-11, short for exercise 1 chapter 11) files with different modes, it will accuratley reflect those modes.
Here's what I came up with:
#!/usr/bin/perl
use warnings;
use strict;
sub filetest {
my @answer;
print "\n\$_[0] now holds $_[0] \n";
return "File does not exist\n"
unless -e $_[0];
push @answer, "readable "
if -r $_[0];
push @answer, "writable "
if -w $_[0];
push @answer, "executable "
if -x $_[0];
print "function result: ";
print @answer;
return @answer;
}
while (@ARGV) {
my $file = shift @ARGV;
$file =~ s/\s*$//;
print "\$file is $file";
my @answer = filetest($file); #thanks for the style tip!
print "\nmain answer: ";
print @answer;
print "\n";
}
Where I was falling down (mainly) was in the subroutine where I was confusing $_ with $_[0]. It was a few chapters ago, and sort of leaked out of my brain. There's a lot to retain to this Perl stuff! ;)
Just wanted to dash off this note as a thanks to everyone who commented. I will try to finish up this program and begin the next one on the train into NYC tommorow.
G'night, Monks! | [reply] [d/l] |
|
|
for ( @ARGV ) {
my @answer = filetest( $_ );
print @answer;
}
| [reply] [d/l] [select] |
Re^2: Unsuccessful stat on file test
by jwkrahn (Abbot) on Mar 02, 2008 at 20:45 UTC
|
You should use s/\s+$//; instead of s/\s*$//;. s/\s*$//; will modify every string that it operates on (every string has zero whitespace in it) while s/\s+$//; will only modify strings that actually have whitespace at the end. | [reply] [d/l] [select] |
|
|
Except that, as was the original problem, every string does have whitespace at the end: the EOL character.
Further, I'm not sure that even if some strings didn't have the EOL whitespace, such as the general case of using s/\s*$// to eliminate whitespace from the end of a string, that this is anything more than premature optimisation. For all I know, perl could already optimise that away, or it may be of no consequence even if it was still "performed". Do you have any evidence that your suggestion improves the OP's code?
Now, if you were to come in and say to use + instead of * because it more literally descriped what the OP was doing, I'd be in much more (but not complete) agreement. That is, the OP wants to replace all whitespaces leading up to the end of the line, qr/\s+$/, with nothing, whereas the code that is currently there could be read as saying that zero-length strings should be replaced, too, which seems silly. It is apparent to anyone with reasonable amounts of regexp-fu that we don't really care about deleting zero-length strings, so it just seems silly to try replacing them. (Conversely, since it's a "don't care" operation, fixing the code now that it's written and actually works doesn't need doing, either.)
I can even think of a pathological case where \s* is better than \s+ - and it's not optimisation (well, not of CPU cycles anyway).
my @filters = ( \&eliminate_ws, \&do_something, \&do_something_else, \
+&etc );
# ...
while (<$myfile>)
{
FILTERS: for my $filter (@filters)
{
# keep filtering until a filter says to stop.
last FILTERS unless $filter->()
}
}
sub eliminate_ws { s/\s*$// } # always returns true.
# as opposed to:
# sub eliminate_ws { s/\s+$//; 1 } # always returns true, even if s "f
+ailed"
It saves the developer a few keystrokes... but, like I said, it's pathological. ;-)
Update: given the test below, I'm even more convinced this is premature optimisation. You're doing 1000 tests per sub call, so asking perl what the savings is comes out as (approximately):
$ perl -e 'printf "%e\n",((1/334000)-(1/1211000))'
2.168248e-06
so we're saving approximately 2 microseconds using the + instead of *. Seriously, that's not significant unless you really are running 1000's (or 100's of 1000's) of times. That's pretty much the definition of premature optimisation.
| [reply] [d/l] [select] |
|
|
$ perl -le'
use Benchmark q/cmpthese/;
my @data = map { join( "", map { ( "a" .. "z" )[ rand 26 ] } 1 .. 10 +
+ rand( 10 ) ) . "\n" } 1 .. 1_000;
cmpthese -10, {
plus => sub {
my @temp = @data;
s/\s+$// for @temp;
return @temp;
},
star => sub {
my @temp = @data;
s/\s*$// for @temp;
return @temp;
}
}
'
Rate star plus
star 336/s -- -66%
plus 992/s 196% --
$ perl -le'
use Benchmark q/cmpthese/;
my @data = map { join( "", map { ( "a" .. "z" )[ rand 26 ] } 1 .. 10 +
+ rand( 10 ) ) . "" } 1 .. 1_000;
cmpthese -10, {
plus => sub {
my @temp = @data;
s/\s+$// for @temp;
return @temp;
},
star => sub {
my @temp = @data;
s/\s*$// for @temp;
return @temp;
}
}
'
Rate star plus
star 334/s -- -72%
plus 1211/s 263% --
It appears that \s+ is always faster than \s*. YMMV
| [reply] [d/l] [select] |
Re^2: Unsuccessful stat on file test
by spork (Monk) on Mar 02, 2008 at 18:41 UTC
|
Just curious Corion...what makes chomp more fragile? | [reply] |
|
|
In my experience, any whitespace at the end of what should be a filename is extraneous.
Often, files which list filenames come with Windows line endings (\x0D\x0A) and on Unix, chomp will only remove the \x0A, leaving remaining but erroneous whitespace.
| [reply] [d/l] [select] |
|
|
Make sense. I have never used chomp when the extra spaces were a factor in processing.Thanks for the tip.
| [reply] |