how can I restart a "do-while" loop after breaking out of a foreach loop?

Question

I have a small snippet of code from my config script, the idea is the configuration is loaded then each key is checked for the hostname that has been entered. But if a configuration is found to contain the same hostname then it is rejected and displays a warning message that a configruation with that hostname already exists.

The problem is I need the foreach loop that checks for the existance of the hash key to restart the do-while loop so another hostname can be tried or the user can ^C out of the script.

Here is the snippet;

my $host;
do {
    print "Enter the hostname or IP of the ESXi server: ";
    chomp($host = <STDIN>);

    if ($host eq '') {
        print "You must enter a hostname or IP address!\n";
    } elsif ($host ne '') {

        # We need to catch duplicate configurations for we don't do the same work twice
        foreach (keys %config) {
            if ($config{$_}{host} ne $host) {
                last;
            } elsif ($config{$_}{host} eq $host) {
                warn "Configuration for $host already exists!\n";
            }
        }

        if ($ping_obj->ping($host)) {
            $config{$config_tag}{host} = $host;
        } elsif (! $ping_obj->ping($host)) {
            print RED . "Ping test for \'$host\' failed" . RESET . "\n";
        }

        $ping_obj->close();
    }
} while ($config{$config_tag}{host} eq 'undef');

This is what the template hash looks like.

my %template = (
    host => 'undef',
    port => 'undef',
    login => {
        user => 'undef',
        password => 'undef',
    },
    options => {
        snapshots => "0",
        compress => "0",

        # This is expressed as an array
        exclude => 'undef',
    },
);

Answer 1

I'm not sure why you are trying to use do ... while, when while seems more natural.

Some notes:

• You do not need to double check your if-statements. If e.g. $host eq '' is true, then $host ne '' must be false. Per definition.
• If you are not going to use $host outside the loop, which I assume you are not, since you store it in the hash, you should put my $host inside the loop, to restrict the scope.

Some tips:

• You can use redo to restart the loop.
• You can use smart matching to do away with the for loop.

---

while ($config{$config_tag}{host} eq 'undef') {
    print "Enter the hostname or IP of the ESXi server: ";
    chomp(my $host = <STDIN>);
    if ($host eq '') {
        print "You must enter a hostname or IP address!\n";
        redo;
    } else {
        # We need to catch duplicate configurations 
        my @host_list = map { $_->{host} } values %config 
        if ($host ~~ @host_list) {
            warn "Configuration for $host already exists!\n";
            redo;
        }
    }
    if ($ping_obj->ping($host)) {
        $config{$config_tag}{host} = $host;
    } else {
        print RED . "Ping test for \'$host\' failed" . RESET . "\n";
    }
    $ping_obj->close();
}

Answer 2

Why do you have 3 elsifs where a simple else would do?
I mean, they only test for the exact opposite of what the associated if tested for.

if ($host eq '') {
    ...
} elsif ($host ne '') {
    ...
}

if ($config{$_}{host} ne $host) {
    ...
} elsif ($config{$_}{host} eq $host) {
    ...
}

if ($ping_obj->ping($host)) {
    ...
} elsif (! $ping_obj->ping($host)) {
    ...
}

---

I would use a normal while loop, instead of a do{...}while(...) loop.

do{
  RESTART:
  if(...){
    goto RESTART;
  }
}while(...);

vs

while(...){
  if(...){
    redo;
  }
}

---

In this loop you are only using the keys of %config to find the associated value, so why don't you use values %config instead.

foreach (keys %config) {
    if ($config{$_}{host} ne $host) {
        last;
    } elsif ($config{$_}{host} eq $host) {
        warn "Configuration for $host already exists!\n";
    }
}

vs

for( values %config ){
  if( $_->{host} ne $host ){
    ...
  } else {
    ...
  }
}

If you're using 5.10.0 or later you could use a smart match (~~) instead, which would make it clearer what you're testing.

my @hosts = map{ $_->{host} } values %config;
if( $host ~~ @hosts ){
  ...
}

Answer 3

If there is ever a use for a goto LABEL statement in Perl, this is it.

do {
    START:     # could also go right before the "do"
    ...
    if (...) {
        warn "Configuration exists. Start over.\n";
        goto START;
    }
} while (...);