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

Does anything else call get_next_file? Do any subclasses override get_next_file? If both of those are "no", get_next_file could be merged into AUTOLOAD, improving performance by avoiding a extra method call for each file.

Independent iterators should not be much of a problem, although the semantics of ->selected_file get "interesting": it would only return the file most recently produced from the most-recently-used iterator, which could lead to confusion. The get_TYPE_files methods avoid this for programs that need to do their own iteration, but I suggest adding a reset_file_iterator method to allow iteration to be aborted rather than requiring 1 while $collector->get_next_file to be used.

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