HiveBrain v1.2.0
Get Started
← Back to all entries
patternModerate

Function, taking up to 27 parameters, that checks for the existence of a path in a graph

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
paththegraphchecksexistencefunctionthatforparameterstaking

Problem

There's got to be a better way than this that preserves the logic while sparing me the multitude of lines:

```
sub has_path {
clearerr;
my %Graph = gref(shift);
my $A = shift;
my $B = shift;
my $C = shift;
my $D = shift;
my $E = shift;
my $F = shift;
my $G = shift;
my $H = shift;
my $I = shift;
my $J = shift;
my $K = shift;
my $L = shift;
my $M = shift;
my $N = shift;
my $O = shift;
my $P = shift;
my $Q = shift;
my $R = shift;
my $S = shift;
my $T = shift;
my $U = shift;
my $V = shift;
my $W = shift;
my $X = shift;
my $Y = shift;
my $Z = shift;
# returns VT_BOOL
my $bool = 0;
my $switcher = dectab( [ $A, $B, $C, $D, $E, $F, $G, $H, $I, $J, $K, $L, $M, $N, $O, $P, $Q, $R, $S, $T, $U, $V, $W, $X, $Y, $Z ] );
given ($switcher) {
when ( "--------------------------" ) {
seterr( "No path." );
} # no path
when ( "X-------------------------" ) {
seterr( "Path of one element." );
} # path of 1 element
when ( "XX------------------------" ) {
$bool = $Graph->has_path( $A, $B );
}
when ( "XXX-----------------------" ) {
$bool = $Graph->has_path( $A, $B, $C );
}
when ( "XXXX----------------------" ) {
$bool = $Graph->has_path( $A, $B, $C, $D );
}
when ( "XXXXX---------------------" ) {
$bool = $Graph->has_path( $A, $B, $C, $D, $E );
}
when ( "XXXXXX--------------------" ) {
$bool = $Graph->has_path( $A, $B, $C, $D, $E, $F );
}
when ( "XXXXXXX-------------------" ) {
$bool = $Graph->has_path( $A, $B, $C, $D, $E, $F, $G );
}
when ( "XXXXXXXX------------------" ) {
$bool = $Graph->has_path( $A, $B, $C, $D, $E, $F, $G, $H );
}
when ( "XXXXXXXXX-----------------" ) {
$bool = $Graph->has_path( $A, $B, $C, $D, $E, $F, $G, $H, $I );
}
when ( "XXXXXXXXXX----------------" ) {
$bool = $Graph->has_path( $A, $B, $C, $D, $E, $F, $G, $H, $I, $J );
}
when ( "XXXXXXXXXXX---------------" ) {
$bool = $Graph

Solution

As I noted in a comment, I think the solution lies in using arrays and slices. Maybe like this:

sub has_path {
  clearerr;
  my %Graph = gref(shift);
  my(@States) = @_;
  my $bool = 0;
  my $switcher = dectab( [ @States ] );
  $switcher =~ m/^(X*)(?:-*)$/;
  my $number = length($1);
  if ($number == 0)  {
    seterr( "No path." );
  }
  else
  {
    $bool = $Graph->has_path( $States[0 .. ($length - 1)] );
  }

  return $bool;
} 

sub dectab {
  my($ref)=shift;
  my ($res);  foreach my $key( @$ref){ 
   if ( ! defined $key ) { 
    $res .= '-';
   } else {
    $res .= "X";
   }
  }
  return $res;
}


The key observations are:

  • The list of letter variables is better treated as an array - I used @States.



  • The output from dectab() (which is unaltered) consists of some number of X's followed by some number of dashes. The regex match identifies how many X's by isolating them into a string, $1, and then calculating the length of the string. Note that the code does not check that the output from dectab matches that pattern - it probably should.



  • The huge switch statement amounts to supplying the elements 0..(N-1) to the $Graph->has_path() function, so the code passes the relevant slice of @States to the function.



There are still some bits I'm not clear about in your code. Specifically, I'm not sure about the roles (or sources) of the functions:

  • clearerr



  • gref



  • seterr



Because of that, I can't test my hypothesis. However, I do think that this solution scales to 200 items more easily than the original - and without needing:

use feature "switch";


With more time spent, the code could still be tidied up, I'm sure. And, since this is Perl, TMTOWTDI - there's more than one way to do it.

Suggestion:

  • Provide code that can be compiled and run whenever possible - you will get better code reviews that way.

Code Snippets

sub has_path {
  clearerr;
  my %Graph = gref(shift);
  my(@States) = @_;
  my $bool = 0;
  my $switcher = dectab( [ @States ] );
  $switcher =~ m/^(X*)(?:-*)$/;
  my $number = length($1);
  if ($number == 0)  {
    seterr( "No path." );
  }
  else
  {
    $bool = $Graph->has_path( $States[0 .. ($length - 1)] );
  }

  return $bool;
} 

sub dectab {
  my($ref)=shift;
  my ($res);  foreach my $key( @$ref){ 
   if ( ! defined $key ) { 
    $res .= '-';
   } else {
    $res .= "X";
   }
  }
  return $res;
}
use feature "switch";

Context

StackExchange Code Review Q#478, answer score: 10

Revisions (0)

No revisions yet.