Thanks again. Yeah, I realize the iterators will stomp on one another. It hasn't been a problem but it's definitely sloppy. Maybe I could set up a hash containing a key for each type of queue. But it might be good enough just to throw an error if the file queue is not empty when a new one is created.

Thanks for the tip on Hash::Util. I was not familiar with restricted hashes. I think the code I currently have to check for an existing property is buggy and it's on my list to test and fix.

Yeah, will definitely use File::Spec if I release this.

For a CPAN release, each category should definitely have an independent iterator. For your own application, throwing an error may be enough.

It might also be better to just return a list of files and let the caller handle iteration with for.

OK, here's what I got for now. I added some efficiency to it and it now throws an error if the user tries to invoke a second iterator before the queue is empty. If I need to nest loops over files, I can use a conventional for loop for that, I guess. Thanks again for the guidance on this.

# Returns files from a method as determined by the "type" in the metho
+d's name:
# get_type_files() returns a list of files found in an obj attribute
# get_next_type_file() places the list of files into a queue for itera
+tion
#
# Examples:
# $s->get_old_files;
# $s->get_next_new_file;

sub AUTOLOAD {
  our $AUTOLOAD;
  my $s = shift;
  $AUTOLOAD  =~ /.*::get(_next)*(_\w+)_files*$/ or
    croak "No such method: $AUTOLOAD";
  my ($next, $type) = ($1, $2);

  # Don't stomp on an already existing iterator
  my $last = $s->{_last_next_req};
  if ($next && $last && ($last ne $type) && @{$s->{_file_queue}}) {
    croak "File queue not empty, cannot create iterator. Aborting.";
  }
  $s->{_last_next_req} = $type if $next;

  # Don't re-fetch files if we already have files in queue
  if ($next && @{$s->{_file_queue}}) {
    return $s->get_next_file;
  }

  # Get the files and return appropriate file(s) based on method calle
+d
  my $attr = "${type}_files";
  my @files = @{$s->{$attr}};
  return @files if !@files || !$next;
  return $s->get_next_file(@files);
}


sub new {
  my $class = shift;

  my $s = bless { _files => {},
                  _target_repo   => '',
                  _selected_file => '',
                  _common_dir    => '',
                  _last_next_req => '',
                  _file_queue    => []}, $class;
  $s->add_resources(@_);
  return $s;
}

sub get_next_file {
  my $s = shift;

  if (!$s->{_selected_file}) {
    my @files = @_ ? @_ : $s->get_files;
    $s->{_file_queue} = \@files;
  }
  my $next_file = shift @{$s->{_file_queue}};
  $s->{_selected_file} = $next_file;
  return $next_file;
}
[download]

$PM = "Perl Monk's";
$MCF = "Most Clueless Friar Abbot Bishop Pontiff Deacon Curate Priest Vicar";
$nysus = $PM . ' ' . $MCF;
Click here if you love Perl Monks

Ok, thanks.

I really like having the external iterator and I think I'm definitely going to keep it. It helps keep the code clean and super simple to follow and I don't have to pass files around as arguments.

my $s = shift;
while ($s->get_next_bad_file) {
  $s->delete_file;
}

...elsewhere...

sub delete_file {
  my $s = shift;;
  unlink $s->selected_file, 
}
[download]

$PM = "Perl Monk's";
$MCF = "Most Clueless Friar Abbot Bishop Pontiff Deacon Curate Priest Vicar";
$nysus = $PM . ' ' . $MCF;
Click here if you love Perl Monks