in reply to Re^5: getting a while loop to terminate
in thread getting a while loop to terminate

Well, I have yet to use the state feature myself, so I may not be the best judge of that, but in this case, it doesn't gain you anything. If you just instantiate $counter3 normally with my, it will be available to subroutines within the file already, without needing state. It won't hurt anything; it just has no particular effect here.

With regard to your logic for finding a new directory name and creating it, I would be concerned about a different kind of race condition. If the mkdir fails because of permissions, your next line deals with that with die. Otherwise it keeps incrementing the counter until it finds a name that can be created. But what if the mkdir fails for some other reason, like EROFS or ENOSPC? Your loop will continue forever, incrementing and trying to mkdir until you kill the program. That's why my version did the mkdir after my do/while loop, and only used the loop to find an available name. As others pointed out, you still need an error check on the mkdir, just in case some other process creates the directory in that split second between your loop selecting the name and mkdir making it, but that's simpler than trying to specifically catch all the possible errors that mkdir might throw.

The double return is awkward, partly because it should never be possible to trigger the second one. Your while loop can only be exited if $made is true, in which case the first return will be executed, or by die, in which case neither return will be reached. The second return is thus a superfluous line that cannot be reached and looks like a possible typo or something.

One last stylistic note: You don't need double quotes to interpolate a single variable, so these are equivalent: 

    my $filename = "$dir". "/image_". "$counter";
    my $filename = $dir. "/image_". $counter;
    my $filename = "$dir/image_$counter";

[download]

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

Replies are listed 'Best First'.
Re^7: getting a while loop to terminate
by aaron_baugher (Curate) on Apr 18, 2012 at 07:56 UTC

    After giving it a bit more thought, I realized that the only "acceptable" error from mkdir (the only one you want to continue looping after) is EEXIST (File exists). So maybe it makes more sense to loop on that error. That eliminates the need for any flags, or any extra error checking to avoid a race condition: 

    $my $counter = 1;
use Errno qw[ EEXIST ];
sub mk_new_dir {
  while(1){
    my $name = $word . '_' . $counter++;
    if( mkdir $name, 0755 ){
      return $name;  # success, return new dir name
    } else {
      next if $!{EEXIST}; # mkdir failed because the file exists; loop
+ and try next name
      die $!;             # it failed for some other reason; bail out!
    }
  }
}

    [download]

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

[reply]
[d/l]

      I thought I posted the final version of this. I've been reading as I come here and noticed that it's a possibility that if your thread dithers on too long, someone might just cut it off. I don't know whether that happened, or I just forgot to click the create button, but I don't think a thread has finished until one has a version with a bit of polish:

      $ perl tg6.pl
downloaded 83 images from http://www.yahoo.com
to folder site_4
$ cat tg6.pl
#!/usr/bin/perl -w
# creates a new directory and downloads images from url to it
# perlmonks node 965537; thx aaron and A.M.
use strict;
use feature ':5.10';
use WWW::Mechanize;
use LWP::Simple;
use Errno qw[ EEXIST ];

# get information about images
my $domain = 'http://www.yahoo.com';
my $m = WWW::Mechanize->new(); 
$m->get( $domain);
my @list = $m->images();

# create new folder and download images to it.
my $counter = 0;
my $dir = &mk_new_dir;
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"; 
}

# output
print "downloaded ", $counter, " images from ", $domain, "\n";
print "to folder ", $dir, "\n"; 

sub mk_new_dir {
  my $counter2 = 1;
  while(1){
    my $word = "site";
    my $name = $word . '_' . $counter2++;
    if( mkdir $name, 0755 ){
      return $name;  # success, return new dir name
    } else {
      next if $!{EEXIST}; # mkdir failed because the file exists
      die $!;             # it failed for some other reason; bail out!
    }
  }
$

      [download]

      I think aaron's integrating the 'mkdir $name, 0755' as the truth condition of the sub is great, as well as the post-increment of $counter2 to drive the thing forward.

      }
[reply]
[d/l]

        I prefer to use  die sprintf "(%d) %s", $!, $!; because the frequent Unknown Error isn't very useful :)

[reply]
[d/l]

In Section Seekers of Perl Wisdom