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

Expand hostnames from a string of hostnames and/or regex

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

Problem

This code will be passed a string which will contain one or more hostnames.

A hostname can be

  • dotted-decimal



  • plain name (like my_host)



  • FQDN or even partially qualified



  • A string containing regex (my_hos[tes])



target_list is an array of known hosts to match any regex against.

Any suggestions on cleaning this up?

sub expand_names {
    my $host = shift;

    my @raw_hosts = split(' ', $host);
    my @hosts;

    my %host_hash;
    foreach my $raw_name (@raw_hosts) {
        $host_hash{$raw_name} = {};
        #if the name might contain regex, let's see what it matched
        if ($raw_name !~ m/^[a-zA-Z0-9-]+$/) {
            foreach my $target (@target_list) {
                if ($target =~ m/^$raw_name$/) {
                    $host_hash{$raw_name}->{$target} = 1;
                }
            }
            #if we didn't match anything, then just use the name
            if (not keys %{$host_hash{$raw_name}} ) {
                $host_hash{$raw_name}->{$raw_name} = 1;
            }
        }
        else {
            # Regular host name or dotted-decimal
            $host_hash{$raw_name}->{$raw_name} = 1;
        }
    }
    # Gather all the hosts we found
    foreach $raw_name  (keys %host_hash) {
        @hosts = (@hosts, (keys %{$host_hash{$raw_name}}));
    }
    return sort @hosts;
}

Solution

Your function expects a single argument that should contain hostnames or patterns separated by spaces:

sub expand_names {
    my $host = shift;
    my @raw_hosts = split(' ', $host);
    foreach my $raw_name (@raw_hosts) {


Why not use multiple arguments?

sub expand_names {
    foreach my $raw_name (@_) {


Since @hosts is empty when you reach this code:

# Gather all the hosts we found
foreach $raw_name  (keys %host_hash) {
    @hosts = (@hosts, (keys %{$host_hash{$raw_name}}));
}


You could simplify:

foreach $raw_name  (keys %host_hash) {
    push(@hosts, keys %{$host_hash{$raw_name}});
}


And actually, I don't see the reason for building a map of maps, when a simple flat map will do.
Consider this alternative implementation:

sub expand_names {
    my %hosts;
    foreach my $raw_name (@_) {
        #if the name might contain regex, let's see what it matched
        if ($raw_name !~ m/^[a-zA-Z0-9-]+$/) {
            my $found = 0;
            foreach my $target (@target_list) {
                if ($target =~ m/^$raw_name$/) {
                    $found = $hosts{$target} = 1;
                }
            }
            $hosts{$raw_name} = 1 if not $found;
        } else {
            # Regular host name or dotted-decimal
            $hosts{$raw_name} = 1;
        }
    }
    return sort keys %hosts;
}

Code Snippets

sub expand_names {
    my $host = shift;
    my @raw_hosts = split(' ', $host);
    foreach my $raw_name (@raw_hosts) {
sub expand_names {
    foreach my $raw_name (@_) {
# Gather all the hosts we found
foreach $raw_name  (keys %host_hash) {
    @hosts = (@hosts, (keys %{$host_hash{$raw_name}}));
}
foreach $raw_name  (keys %host_hash) {
    push(@hosts, keys %{$host_hash{$raw_name}});
}
sub expand_names {
    my %hosts;
    foreach my $raw_name (@_) {
        #if the name might contain regex, let's see what it matched
        if ($raw_name !~ m/^[a-zA-Z0-9-]+$/) {
            my $found = 0;
            foreach my $target (@target_list) {
                if ($target =~ m/^$raw_name$/) {
                    $found = $hosts{$target} = 1;
                }
            }
            $hosts{$raw_name} = 1 if not $found;
        } else {
            # Regular host name or dotted-decimal
            $hosts{$raw_name} = 1;
        }
    }
    return sort keys %hosts;
}

Context

StackExchange Code Review Q#61464, answer score: 6

Revisions (0)

No revisions yet.