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

Perl Monks of wisdom,
This is my first post, as I am a complete perl novice. I have written a script utilizing the Net:Snmp module to query load balancing switches. Could some of you please comment on the coding style with suggestions on brevity, alterations and the like? Any comments are greatly appreciated. 
#!/usr/bin/perl -w
use NET::SNMP;
use strict;
use vars qw($session $error $response $syscontact $table $oid);

Usage() unless(defined $ARGV[0]);
Usage() unless(defined $ARGV[1]);

($session, $error) = Net::SNMP->session(
    -hostname => $ARGV[0],
    -community => $ARGV[1]
);

die "session error: $error" unless ($session);

my $tableid = "1.3.6.1.4.1.1991.1.1.4.6.1.1";
$table = $session->get_table($tableid);
die "request error: ".$session->error unless (defined $table);
foreach $oid (keys %$table) {
 if ($oid=~/1.3.6.1.4.1.1991.1.1.4.6.1.1.4.[0-9]+/) {
   my @digits = split (/\./, $oid);
   my $enddig = $digits[-1];
   my $rsportoid = "1.3.6.1.4.1.1991.1.1.4.6.1.1.5.$enddig";
   my $vsnameoid = "1.3.6.1.4.1.1991.1.1.4.6.1.1.2.$enddig";
   my $vsportoid = "1.3.6.1.4.1.1991.1.1.4.6.1.1.3.$enddig";
   print "Real server ".$table->{"$oid"}.", port ".$table->{"$rsportoi
+d"}.", is bound to virtual server ".$table->{"$vsnameoid"}.", on port
+ ".$table->{"$vsportoid"}."\n";
    }
}

$syscontact = $session->get_request("1.3.6.1.2.1.1.4.0");
die "request error: ".$session->error unless (defined $syscontact);
print "For help or questions contact: ".$syscontact->{"1.3.6.1.2.1.1.4
+.0"}."\n";

$session->close;

sub Usage{
 print(
   "\n",
   "  You didn't provide target hostname or IP address and community s
+tring!\n",
   "  Usage: snmp1.pl host_ip|name community_string\n",
   "  Net::SNMP       $Net::SNMP::VERSION\n");
   exit 1;
   }

[download]
Thank you for your time,
Jer

Replies are listed 'Best First'.
Re: Net::Snmp Attempt
by Abigail (Deacon) on Jul 12, 2001 at 01:30 UTC
    I would write is as follows: 
    #!/usr/bin/perl -w
     
use strict;
use Net::SNMP;

Usage () unless @ARGV == 2; 

my ($session, $error) = Net::SNMP -> session (
    -hostname  => $ARGV [0],
    -community => $ARGV [1],
);

$session or die "Session error: $error";
                
my $tableid = "1.3.6.1.4.1.1991.1.1.4.6.1.1";
my $table   =  $session -> get_table ($tableid)
                or die "request error: ", $session -> error;
foreach my $oid (keys %$table) {
    if ($oid =~ /^\Q$tableid\E\.(\d+)/) {
        print "Real server ",                  $table -> {$oid},
              ", port ",                       $table -> {"$tableid.5.
+$1"},
              ", is bound to virtual server ", $table -> {"$tableid.2.
+$1"},
              ", on port ",                    $table -> {"$tableid.3.
+$1"},
              "\n";
    }
}

my $sysid      = "1.3.6.1.2.1.1.4.0";
my $syscontact = $session -> get_request ($sysid)
                  or die "Request error: ", $session -> error;
print "For help or questions contact: ", $syscontact -> {$sysid}, "\n"
+;

$session -> close;

    [download]
    Some differences: no global variables (with use vars). No magical constants that are repeated. Taken care of the special meaning of a dot inside a regex. Anchored the regex. Used parenthesis to capture relevant field. No needless concatenation in print or die. Success is checked right away and not in a separate statement. Just one check to see if there are enough arguments.

    -- Abigail

[reply]
[d/l]
[select]
      Abigail,

      Thanks for your help. I understand most of the changes you made. I especially appreciate the regex cleanup. I'm puzzled by the lines

      $table -> {"$tableid.5.$1"},
$table -> {"$tableid.2.$1"},

      [download]

      What does the $1 do?

      Jer

[reply]
[d/l]
        $1 doesn't really "do" anything; it's a variable having a value. It is set by the preceeding regular expression which contains a set of parenthesis. The part of the string that is matched by the sub expression in the parens is put in $1. This avoids needing to do the split and getting the last element. See also the perlre manual page.

        -- Abigail

[reply]
[d/l]
[select]

Back to Seekers of Perl Wisdom