use Perl

• My head still hurts (Score:1)

  by Aristotle (5147) <pagaltzis@gmx.de> on 2009.02.01 23:04 (#67166) Homepage Journal

  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.

  Reply to This