I have an ugly Perl subroutine that has a nested if statement that returns different values depending on the conditions. It's ugly because I don't like nested if statements. They're hard to follow so I want to break it up into separate if statements but I'm not sure if it'll change the way the subroutine performs. The original subroutine follows:
sub TacacsInstalled(){
my $Installed;
my $InstalledVersion;
my $InstalledRelease;
my $NewVersion;
my $NewRelease;
$Installed = `rpm -qa | grep tac_plus | wc -l`;
chomp $Installed;
# Check to see if Tacacs is installed.
if ($Installed == 0){ # Tacacs is not installed. Fresh install.
return "False";
} else { # Tacacs is installed. Is it an old version?
$InstalledVersion = `rpm -q tac_plus --queryformat \"%{VERSION}\""`;
chomp $InstalledVersion;
$NewVersion = `rpm -qp ./tac_plus*.x86_64.rpm --queryformat \"%{VERSION}\""`;
chomp $NewVersion;
if ($InstalledVersion < $NewVersion) { # Current installed version is too old, must update.
return "False";
} else { # Maybe the release is newer.
$InstalledRelease = `rpm -q tac_plus --queryformat \"%{RELEASE}\""`;
chomp $InstalledRelease;
$NewRelease = `rpm -qp ./tac_plus*.x86_64.rpm --queryformat \"%{RELEASE}\""`;
chomp $NewRelease;
if ($InstalledRelease < $NewRelease) { # Current installed release is too old, must update.
return "False";
} else { # Installed release is newer than or equal to what we're trying to install. Do nothing.
return "True";
}
}
}
}
Could I improve this code by using the following instead?
sub TacacsInstalled(){
my $Installed;
my $InstalledVersion;
my $InstalledRelease;
my $NewVersion;
my $NewRelease;
$Installed = `rpm -qa | grep tac_plus | wc -l`;
chomp $Installed;
# Check to see if Tacacs is installed.
if ($Installed == 0){ return "False" }; # Tacacs is not installed perform a fresh install.
# If we got this far then Tacacs must be installed. Is it an old version?
$InstalledVersion = `rpm -q tac_plus --queryformat \"%{VERSION}\""`;
chomp $InstalledVersion;
$NewVersion = `rpm -qp ./tac_plus*.x86_64.rpm --queryformat \"%{VERSION}\""`;
chomp $NewVersion;
if ($InstalledVersion < $NewVersion) { return "False" }; # Current installed version is too old, must update.
# The Version is the same so then is the release is newer?
$InstalledRelease = `rpm -q tac_plus --queryformat \"%{RELEASE}\""`;
chomp $InstalledRelease;
$NewRelease = `rpm -qp ./tac_plus*.x86_64.rpm --queryformat \"%{RELEASE}\""`;
chomp $NewRelease;
if ($InstalledRelease < $NewRelease) { return "False" }; # Current installed release is too old, must update.
# Installed release is newer than or equal to what we're trying to install. Do nothing.
return "True";
}
No, Perl does not execute code after a return
. That's the whole point of return
!