Drinking coffee. Hacking diligently. This code worked perfectly the first time I wrote it. I know what it's supposed to do, but I still can't believe it works. When my own code gives me a headache, I think I've made a mistake.
# This method builds 'paths'. These are the paths the inheritance hierarchy
# will take through the code to find a method. This is based on Perl's
# default search order, not C3.
sub _build_paths {
my ( $self, $class, @parents ) = @_;
my @paths = $self->paths;
for my $i ( 0 .. $#paths ) {
my $path = $paths[$i];
if ( $path->[-1] eq $class ) {
my @new_paths = map { [@$_] } ($path) x @parents;
for my $j ( 0 .. $#parents ) {
push @{ $new_paths[$j] } => $parents[$j];
}
splice @paths, $i, 1, @new_paths;
$self->paths(@paths);
}
}
}
Yeah, let's try that again.
sub _build_paths {
my ( $self, $class, @parents ) = @_;
my @paths = $self->paths;
for my $i ( 0 .. $#paths ) {
my $path = $paths[$i];
if ( $path->[-1] eq $class ) {
my @new_paths;
for my $parent_class (@parents) {
push @new_paths => [@$path, $parent_class];
}
splice @paths, $i, 1, @new_paths;
$self->paths(@paths);
}
}
}
Much better. I still have no idea why the first way popped into my damned head.
First of all:
sub _build_paths {
my ( $self, $class, @parents ) = @_;
my @paths = $self->paths;
for my $i ( 0.. $#paths ) {
my $path = $paths[$i];
if ( $path->[-1] eq $class ) {
my @new_paths = map { [@$path, $_] } @parents;
splice @paths, $i, 1, @new_paths;
$self->paths(@paths);
}
}
}
But that is still terrible, if you’ll excuse my frankness. Somewhere in the middle of a loop you surreptitiously modify the array you are iterating over with splice. Is the fact that you might end up over- or undershooting the end of the array (because the list of indices was generated before modification and @parents may contain 0 or more than 1 elements) intentional? What does it mean? It has all the readability of GOTO spaghetti…
As an aside, you call a mutator potentially over and over, even though there seems to be no reason you couldn’t call it just once.
I think you meant something close to the following:
sub _build_paths {
my ( $self, $class, @parents ) = @_;
my $do_upd;
my @paths = map {
my $path = $_;
$path->[-1] eq $class
? do { ++$do_chg; map { [@$path, $_] } @parents }
: $path
}
} $self->paths;
$self->paths(@paths) if $do_chg;
}
Or if the mutator call isn’t that expensive and has no other side effects you could just leave out the flag entirely.
Arg! You're right, I totally biffed that one. It took me long enough to realize the general shape of the algorithm that I just totally missed what I was iterating over. I've added your code.