Stories
Slash Boxes
Comments
NOTE: use Perl; is on undef hiatus. You can read content, but you can't post it. More info will be forthcoming forthcomingly.

All the Perl that's Practical to Extract and Report

The Fine Print: The following comments are owned by whoever posted them. We are not responsible for them in any way.
 Full
 Abbreviated
 Hidden
More | Login | Reply
Loading... please wait.
  • 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.