Beefy Boxes and Bandwidth Generously Provided by pair Networks
Pathologically Eclectic Rubbish Lister
 
PerlMonks  

Re: getting a while loop to terminate

by Anonymous Monk
on Apr 15, 2012 at 04:15 UTC ( #965122=note: print w/ replies, xml ) Need Help??


in reply to getting a while loop to terminate

 my $m = WWW::Mechanize->new;

Replace with  WWW::Mechanize->new( qw/ autodie 1 /);

 getstore($url,$filename) or die "Can't download '$url': $@\n";

Get rid of LWP::Simple , replace with  $m->mirror( $url, $filename ); autodie takes care of the dying

 if ( -e -d $name ) {

While you can (since 5.9.1) chain/stack file-test operators in a case like this (it's the quivalent of  -d $name && -e _ ) , if its a directory, it already exists (otherwise how can it be a directory ), so if you're going to test to see if its a directory, just use -d

Checking if a directory exists before creating the directory is a classic race condition --- some other program could make the directory after your program checks that it doesn't exist, but before it creates it -- this may or may not matter to your program

Since creating a directory is an atomic operation (either succeed or fail), simply try to create the directory -- if it already exists, creation will fail

Here is how I might write that

sub Dirpp { use Errno qw/ EACCES /; # permission denied my $newdir = "site00"; my $made = 0; while ( not $made = mkdir $name, 0755 ) { die $! if $! == EACCES; $newdir++; } return $name if $made; # if we made it, return new name return; }

This takes advantage of the magical string increment or auto-increment. auto-increment does overflow, so if that isn't desired, then I would write

sub DirPP { use Errno qw/ EACCES /; # permission denied my $word = "site"; my $counter = 1; my $name ; my $made = 0; while(1){ $name = sprintf '%s%3d', $word, $counter; last if not $made = mkdir $name, 0755 ; die $! if $! == EACCES; } return $name if $made; return; }

Why  while(1) ? Because to me two sprintf looks clumsy

sub DirPo { use Errno qw/ EACCES /; # permission denied my $word = "site"; my $counter = 1; my $made = 0; my $name = sprintf '%s%3d', $word, $counter; ## ONE, PEE while ( not $made = mkdir $name, 0755 ) { die $! if $! == EACCES; $name = sprintf '%s%3d', $word, $counter; ## TWO, EEW } return $name if $made; return; }

Also, storing one file per directory is bizzare :)


Comment on Re: getting a while loop to terminate
Select or Download Code
Re^2: getting a while loop to terminate
by Anonymous Monk on Apr 15, 2012 at 05:03 UTC

    To avoid duplicate checks during the run of a program, I would optimize by making $counter a state variable

    The modern perl way

    use feature qw/ state /; state $counter = 1;

    Or the old way, with a closure

    BEGIN { my $counter = 1; sub DirPP { use Errno qw/ EACCES /; # permission denied my $word = "site"; my $name ; my $made = 0; while(1){ $name = sprintf '%s%3d', $word, $counter; last if not $made = mkdir $name, 0755 ; die $! if $! == EACCES; } return $name if $made; return; } }

      I'm really having a hard time putting responses where they need to be. Since my re-working of what aaron posted and what AM posted are catching in the same place, I hope that putting the responses here doesn't offend.

      I had two big problems in the original post that have now been corrected: single quotes around the zero for false and calling for a new DirPP while in the loop that downloads the images.

      Now I'm trying to integrate the slicker versions of DirPP and stumble on the same error despite which version I try:

      $ perl tg2.pl Unrecognized LWP::UserAgent options: autodie at tg2.pl line 7 Use of uninitialized value $dir in string at tg2.pl line 16. Use of uninitialized value $dir in string at tg2.pl line 16. ... Use of uninitialized value $dir in string at tg2.pl line 16. ^C $ cat tg3.pl #!/usr/bin/perl -w use strict; use WWW::Mechanize; use LWP::Simple; use feature qw/ state /; my $domain = 'http://www.yahoo.com'; my $m = WWW::Mechanize->new( qw/ autodie 1 /); $m->get( $domain); my $counter = 0; my @list = $m->images(); my $dir = &DirPP; for my $img (@list) { my $url = $img->url_abs(); $counter++; my $filename = "$dir". "/image_". "$counter"; #line 16 getstore($url,$filename) or die "Can't download '$url': $@\n"; } sub DirPP { use Errno qw/ EACCES /; # permission denied my $word = "site"; my $counter = 1; my $name ; my $made = 0; while(1){ $name = sprintf '%s%3d', $word, $counter; last if not $made = mkdir $name, 0755 ; die $! if $! == EACCES; } return $name if $made; return; } $

      From the look of the output, the subroutine is being called many times, but to my eye, I call it only once, and that is before I enter the loop that assigns a value to $dir . (scratches head) That was aaron's version, but I induce the same behavior with AM's version:

      $ perl tg2.pl Unrecognized LWP::UserAgent options: autodie at tg2.pl line 7 Use of uninitialized value $dir in string at tg2.pl line 15. Use of uninitialized value $dir in string at tg2.pl line 15. Use of uninitialized value $dir in string at tg2.pl line 15. Use of uninitialized value $dir in string at tg2.pl line 15. Use of uninitialized value $dir in string at tg2.pl line 15. Use of uninitialized value $dir in string at tg2.pl line 15. Use of uninitialized value $dir in string at tg2.pl line 15. Use of uninitialized value $dir in string at tg2.pl line 15. ^C $ cat tg2.pl #!/usr/bin/perl -w use strict; use WWW::Mechanize; use LWP::Simple; use feature ':5.10'; my $domain = 'http://www.yahoo.com'; my $m = WWW::Mechanize->new( qw/ autodie 1 /); $m->get( $domain); my @list = $m->images(); my $counter = 0; my $dir = &DirPP; for my $img (@list) { my $url = $img->url_abs(); $counter++; my $filename = "$dir". "/image_". "$counter"; getstore($url,$filename) or die "Can't download '$url': $@\n"; } sub DirPP { use Errno qw/ EACCES /; # permission denied state $counter2 = 1; my $word = "site"; my $name ; my $made = 0; while(1){ $name = sprintf '%s%3d', $word, $counter2; last if not $made = mkdir $name, 0755 ; die $! if $! == EACCES; } return $name if $made; return; } $

      So, I'm stumped and out of guesses. That means I have no excuse to stay on the computer...off to the world of work..., happy tuesday,

        The error message isn't coming from within your subroutine; it's coming from within your loop through the images, at line 15. It's telling you that the $dir variable is undefined on each trip through the loop, although you're correct that you only assigned to it once, before the loop.

        I only have time for a quick glance right now, but I suspect that your "last if not $made" logic has gotten things a little confused. To me, that says to break out of the loop if the directory was NOT made. If it was NOT made (which would be the case if your first attempt at picking a name picked one that already existed), then $made will be false, so your first return will not trigger, and your second return will return undef, which is assigned to $die. You probably want "last if $made", because you want to break out of the loop once the directory is created, right?

        Aaron B.
        My Woefully Neglected Blog, where I occasionally mention Perl.

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://965122]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others lurking in the Monastery: (16)
As of 2014-09-17 16:00 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    How do you remember the number of days in each month?











    Results (90 votes), past polls