walkingthecow has asked for the wisdom of the Perl Monks concerning the following question:

I have a subroutine that creates an SSH connection to a specified server using Expect. My subroutine is huge and ugly due to the fact that I have so many nested if-elsif-else statements. I am wondering what may be a better way to do this. Modified (for length) code below (notice all the nested if statements under if ( $match_num == 2 ) ): 
sub connect {
    my $host = shift;
    my $SSH = Expect->spawn( @ssh, "$user\@$host" );
    ( $match_num, $error, $match, $before, $after ) =
      $SSH->expect(    $time,
                    '-re', '[Nn]ew [Pp]assword:',
                    '-re', '^[Pp]assword.*:',            
                    '-re',    $prompt
      );
    if ($match_num == 1) {
        $result = "FAILED: Account Expired";
    }
    elsif ( $match_num == 2 ) {
        print $SSH "$pass\r";
        ( $match_num, $error, $match, $before, $after ) =
          $SSH->expect(    $time,
                        '-re', '[Pp]assword.*:',
                        '-re',    $prompt
          );
        if ( $match_num == 1) {
            $result = "FAILED: Incorrect Password";
        }
        elsif ( $match_num == 2 ) {
            print $SSH "export PS1='scripts: '\r";
            ( $match_num, $error, $match, $before, $after ) = 
              $SSH->expect( $time, 
                            '-re', '\r\nscripts: ' );
            if ( $match_num == 1 ) {
                $result = "SUCCESS: you are logged into $host";
            }
            else {
                $before =~ s/\r\n//g;
                $after  =~ s/\r\n//g;
                $result = "ERROR:  $error  BEFORE: $before  AFTER: $af
+ter";
            }
        }
        elsif ( $match_num == 3) {
            $result = "SUCCESS: you are logged into $host";
        }
    }
    elsif ( $match_num == 3) {
        $result = "SUCCESS: you are logged into $host";
    }
    else {
        $before =~ s/\r\n//g;
        $after  =~ s/\r\n//g;
        $result = "ERROR:  $error  BEFORE: $before  AFTER: $after";
    }
}

[download]

Replies are listed 'Best First'.
Re: Fixing cascading if-elsif-else statements
by ww (Archbishop) on Aug 28, 2009 at 01:17 UTC
[reply]
Re: Fixing cascading if-elsif-else statements
by GrandFather (Saint) on Aug 28, 2009 at 01:25 UTC

    Deal with trivial cases first and use early exits. Consider:

    sub connect {
    my $host = shift;
    my $SSH = Expect->spawn (@ssh, "$user\@$host");
    my ($match_num, $error, $match, $before, $after) =
        $SSH->expect ($time, '-re', '[Nn]ew [Pp]assword:',
        '-re', '^[Pp]assword.*:', '-re', $prompt);
    if ($match_num == 1) {
        $result = "FAILED: Account Expired";
        return;
    }

    if ($match_num == 3) {
        $result = "SUCCESS: you are logged into $host";
        return;
    }

    if ($match_num != 2) {
        $before =~ s/\r\n//g;
        $after  =~ s/\r\n//g;
        $result = "ERROR:  $error  BEFORE: $before  AFTER: $after";
        return;
    }

    print $SSH "$pass\r";
    ($match_num, $error, $match, $before, $after) =
        $SSH->expect ($time, '-re', '[Pp]assword.*:', '-re', $prompt);

    if ($match_num == 1) {
        $result = "FAILED: Incorrect Password";
        return;
    }

    if ($match_num == 3) {
        $result = "SUCCESS: you are logged into $host";
        return;
    }

    return if ($match_num != 2);

    print $SSH "export PS1='scripts: '\r";
    ($match_num, $error, $match, $before, $after) =
        $SSH->expect ($time, '-re', '\r\nscripts: ');

    if ($match_num == 1) {
        $result = "SUCCESS: you are logged into $host";
        return;
    }

    $before =~ s/\r\n//g;
    $after  =~ s/\r\n//g;
    $result = "ERROR:  $error  BEFORE: $before  AFTER: $after";
}

    [download]
    True laziness is hard work
[reply]
[d/l]
      Awesome, thank you! I ended up using a dispatch table for this, but I also liked the suggestion of just returning $result on failure.
[reply]
Re: Fixing cascading if-elsif-else statements
by ganeshk (Monk) on Aug 28, 2009 at 07:13 UTC
    Please take a look at Switch statements if you are wanting to make the code readable. But you need perl 5.10 for doing this.
[reply]

Back to Seekers of Perl Wisdom