Every iterator subclass has a corresponding subclass of File::Collector that sorts files into additional categories. I had expected that each subclass that adds attributes ends up adding them to the same hashref as is indexed by $collector->{files}, so that if App::whatever::Collector::HeaderAnalyzer adds an is_valid_header attribute, that attribute appears in $collector->{files}{$filename}{is_valid_header}. Since the iterator now has the entire ->{files}{$filename} hash in its array, the AUTOLOAD magic that I suggested would provide a ->selected_file_is_valid_header method on the iterator object with no actual code in the iterator subclass.

Nice, I got the File::Collector subclass down to one sub and just a few lines of code now:

package File::Collector::DateClassifier ;

use strict; use warnings;
use Log::Log4perl::Shortcuts qw(:all);
use File::Collector::DateClassifier::Iterator;
use parent qw ( File::Collector );

sub _classify_files {
  my ($s, $files_added) = @_;

  $s->add_iterators( qw ( some_files other_files ) );
  foreach my $file (sort @$files_added) {
    $s->add_to_iterator('some_files', $file);
  }
}

return 1;
[download]

This feels fun, now. I feel like I can call myself an intermediate Perl programmer now. Only took 20 years. :)

Thanks again for all your time advising me on this. I learned a lot. I'll post this up to CPAN when I think it's CPAN worthy. I got it on a git for now: File::Collector

$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

Nice, but I think you could eliminate lines 4 and 5 also, if you keep the corresponding ::Iterator subclass in the same file, and I do not see any log4perl calls here.

And the convention for the final true value is a simple 1;, with no need for the return keyword.

And you are very welcome. There are still some rough edges (as I mentioned in the other reply) and I will say to make sure to write good tests. I have found Devel::Cover useful in my own work, but be careful that you do not end up "testing to the implementation".

Phew, ok, I think I really got it down good now. I did some more refinement of how things work, too. Here's what I got:

package File::Collector::DateAnalyzer ;

use strict; use warnings;
use Log::Log4perl::Shortcuts qw(:all);
use File::Collector::DateAnalyzer::Iterator;
use parent qw ( File::Collector );

sub add_resources {
  my $s = shift;

  $s->SUPER::add_resources(@_);
  $s->{files}{some_files}         = File::Collector::DateAnalyzer::Ite
+rator->new();
  $s->{files}{other_files}        = File::Collector::DateAnalyzer::Ite
+rator->new();
  $s->_test_blah;
}

sub _test_blah {
  my $s = shift;

  foreach my $file ($s->get_files) {
    $s->{files}{some_files}->add_file($s->{files}{all}{$file});
  }

}

return 1;



package File::Collector::DateAnalyzer::Iterator ;
use strict;
use warnings;

use parent qw (File::Collector::Iterator);
use Log::Log4perl::Shortcuts       qw(:all);

sub print_blah_names {
  my $s = shift;
  print $s->next->{short_path} . "\n\n";
}


1;

package File::Collector::Iterator ;
use strict;
use warnings;

use Carp;
use Log::Log4perl::Shortcuts       qw(:all);

sub new {
  my $class = shift;
  bless [@_], $class;
}

sub next {
  my $s = shift;
  my $last = shift @$s;
  push @$s, $last;
  return  $last;
}

sub add_file {
  my $s = shift;
  pop @$s;
  push @$s, shift;
  push @$s, '';
}

sub print_short_names {
  my $s = shift;
  print $s->next->{short_path} . "\n";
}

sub selected_file {
  my $s = shift;
  $s->[0];
}

sub do {
  my $self = shift;
  bless \$self, 'File::Collector::Iterator::All';
}

sub DESTROY {
  my $s = shift;
  $s->next while ($s->selected_file);
}

{
  package File::Collector::Iterator::All;
  use Log::Log4perl::Shortcuts       qw(:all);
  sub AUTOLOAD {
    our $AUTOLOAD;
    my $self = shift;
    my @method = split /::/, $AUTOLOAD;
    my $method = pop @method;
    $$self->$method(@_) while ($$self->selected_file);
    $$self->next(@_);
  }
}
[download]

A few notes:

Iterator is now a circular queue, terminated with an empty string.

Attributes of File::Collector subclasses are now iterators themselves. This let me get rid of the AUTOLOAD in File::Collector.

I restrucutred File::Collector->{files} as you suggested.

This simple test works:

{
  my $da;

  # 1
 lives_ok { $da = File::Collector::DateAnalyzer->new('t/test_data/many
+_files'); }
   'creates DateAnalyzer object';

 $da->{files}{some_files}->do->print_blah_names;
 $da->{files}{some_files}->do->print_short_names;
}
[download]

I will improve upon the do method calls so I don't have the ugly hashes in there and will use a simple AUTOLOAD so I can do $da->some_files->do->print_blah_names

How does it look to you?

$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

For a general-purpose File::Collector, I would expect support for multiple iterators even over the same category. ...

And it just dawned on me: since Iterators are now circular (and presumably the empty string is not a valid file name, although it could be replaced with undef if someone tries this on a platform where it is valid and complains), all you need to support independent iterators for applications that want them is a ->clone method on iterators:

sub clone {
  my $self = shift;
  bless [@$self], ref $self;
}
[download]

And you have made a change that is going to bite your users with infinite loops: the AUTOLOAD for ::Iterator::All should advance the iterator itself: $$self->$method(@_) while ($$self->next); or $$self->method(@) while (defined $$self->next); if you choose to make the end-of-set marker undef instead of an empty string. The method called by invoking ->do should not advance the iterator itself, but only use $self->selected_file.