in reply to Re^6: Advice on transforming a sub into a re-entrant recursive-able method.
in thread Advice on transforming a sub into a re-entrant recursive-able method.
The thing that immediately pops out is that you have several hard-coded parameters — values which "guide" the function. These should be un-hard-coded, and replaced with parameters you can set with arguments. These are '1.3.6.1.2.1.2.2.1.2', .1.3.6.1.2.1.2.2.1.6', and 'eth0'. The first two may be static for your installation; they could be set by a config file or something. But the 'eth0' is, from what you describe, probably something you want to set via an argument to the function so that it can take on various values during the course of your program run.
Here is my rewrite of your code.
# global config: my @interface_names_oid = ( '1.3.6.1.2.1.2.2.1.2' ); my $interface_oid_prefix = '1.3.6.1.2.1.2.2.1.6'; sub snmp_get($;$) { my( $snmp_sess, $device ) = @_; $device ||= 'eth0'; # default my $interfaces_table = $snmp_sess->get_entries( -columns => \@inte +rface_names_oid ) or die $snmp_sess->error; $snmp_sess->get_request( -varbindlist => [ map { join '.', $interface_oid_prefix, /\.(\d+)$/; } grep { $interfaces_table->{$_} =~ /$device/ } keys %$interfaces_table ] ) }
Note that its return signature is different from yours. This is because my version throws an exception on error, rather than returning an error code. The calling code would need to be modified appropriately.
|
|---|
| Replies are listed 'Best First'. | |
|---|---|
|
Re^8: Advice on transforming a sub into a re-entrant recursive-able method.
by hechz (Novice) on Jul 21, 2008 at 20:30 UTC |