in reply to Re^4: 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.

because sometimes the usefulness of being able to call a function recursively will be a surprise

It doesn't make any sense to try to follow advice if you don't know what it means. Your original function does one fairly simple thing. I think it's highly unlikely that it will ever need to be called recursively. If it turns out that that it will, the function is already in pretty good shape, because it uses no external variables (that is, it accesses no global variables and no external lexical variables). However, it does depend on external state — specifically, the state stored in the snmp session object; and also whatever is read from the snmp space via the snmp session object. These factors suggest that making this function re-entrant may be complicated, and it's quite likely that your little function isn't the right place to solve the problem. On the other hand, the data from snmp space may be relatively static, and the snmp session may be a well-behaved black box (storing no state that would impact your function). If this is the case, then your function is already as recursion-ready as it needs to be. If you didn't figure out all this yourself, then probably you should simply skip that nugget of wisdom from HOP, for now.

See also: voodoo programming and You aren't gonna need it.

Between the mind which plans and the hands which build, there must be a mediator... and this mediator must be the heart.
  • Comment on Re^5: Advice on transforming a sub into a re-entrant recursive-able method.

Replies are listed 'Best First'.
Re^6: Advice on transforming a sub into a re-entrant recursive-able method.
by hechz (Novice) on Jul 21, 2008 at 19:57 UTC
    I agree. The point of altering this routine is to force my self to understand the advice.
    I have been able to follow along with the various and sundry examples of OO-ification, and can use and alter other programmers OO code. In this case I want to make my brain bend enough to begin synthesizing it. My original method of achieving that goal was going to be to just write variants of the code from the book, and to try to use them.

    Then I thought about adding some more queries to my SNMP scan parsing script, and thought to myself "Hmmm, why not try to refactor my current routines to be more generic."
    The statements asserting that I am not quite sure what I am trying to do are fair. In response I can only offer, that I am trying to do something out of my comfort zone to improve my skills. The reason that I am asking for help is that I have failed to achieve that "Aah-ha!" moment, and the underlying mindset of reducing functionality to a base-case has not come.

    So to that end, I shall rephrase. If you, the great and esteemed monks :-), were to write a block of code that could be handed string, and return a hash keyed with interface names with values of MAC addresses, such that the behavior of the lookups of the information and the OIDs changed based on the kind of device the script found, what would be the your first step in dividing this problem into discreet elements of function.

    Below you'll find the entirety of the script, which; I hope, will provide some context to the Monks. moved code to the top-level message

[reply]

      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
    ] )
}

      [download]

      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.

      Between the mind which plans and the hands which build, there must be a mediator... and this mediator must be the heart.
[reply]
[d/l]
[select]
        Admittedly the OIDs and interface name are embedded due to laziness. Usually I have a config.pm that sets global constants and refer to them as $config::interface_oid_base ot some such whosy-whatsit.

        The mapped grep is a great idea, what are your thoughts on making $device the returned list of interfaces available on the device? At my current skill level, I would create a global hash, iterate of the results executing your variant of &snmp_get to return the values. setting the value of $host_interfaces{$device_name}=>($snmpe_result)[0];
        Assuming that an array was returned of course.

[reply]

In Section Seekers of Perl Wisdom