I thought some on this list might be (interested|amused|horrified) at
some code I wrote recently.  Be forewarned, the code in this e-mail is
not meant for those with low tolerances for ugly hacks :)

I wrote this function yesterday, the purpose of which is to ssh to a
remote machine and determine the number of interefaces and their MAC
addresses and return a list of "if mac" pairs.  Normally I'd probably
return a hash, but this function needed to mimic the behavior of
another function which does the same thing by a different means, so I
was trapped into this style of return.

Ironically, what I ended up with, was more or less exactly what I
started out with but there was a lot of refactoring in the middle.

The code here is fairly readable, and straight forward[1].  Which is a
good thing, since it also means it's easily maintainable in the long
run.

######################################################################
# Ping the host and return the MAC address associated with that IP
#
# @param   host     The host
# @return  ifconfig The MAC address for this IP
##
sub getMacs {
  my ($host) = assertNumArgs(1,@_);
  my $ping = Net::Ping->new();

  my $iface = '((?:\w+(?:[0-9]+|:[0-9]+))|lo)'                          ;
  my $macRegex  = 'HWaddr\s([A-Fa-f0-9]{1,2}(?::[A-Fa-f0-9]{1,2}){5})'  ;

  # check if we can get to the host to check it's Mac addresses
  if (!$ping->ping($host)) {
    $log->error("Can not ping $host to obtain MAC Addresses");
    return;
  }

  # find out what interfaces it has from /proc
  my $procInterfaces
    = (assertRemoteCommand($host, "grep : /proc/net/dev", "-l root"))->{stdout};

  # take only the interesting interfaces
  my @Interfaces 
    = grep { s/:.*// && s/^\s+// && !/^lo$/ } split(/\n/, $procInterfaces);

  # get only the interface name/Mac address line
  my @ifconfig = map { (assertRemoteCommand($host,
                        "/sbin/ifconfig $_ | grep HW",
                        "-l root"))->{stdout} } @interfaces;
  chomp(@ifconfig);

  # remove all but the name and Mac address
  map { s/$iface.*$macRegex.*/$1 $2/g} @ifconfig;

  # return a list in the same format as athinfo() does.
  return @ifconfig;
}

What I started out with wasn't too much different from this.  It was,
I think, a little simpler, but some of the original simplicity got
lost in all the refactoring along the way, and, I think, in some
places, even replaced with correctness :)

At one point, I asked a friend to come and review my code.  This
person happens to be a scheme/lisp afficianado as well as one of the
best perl hackers I've ever run into[2].  After about 15 minutes of
hacking, we had refactored the above code into about 3 variable
declarations and a return statement which consisted of a something like
'map {...} grep {...} split (...);' But I found a minor bug in it
later on.  Rather than returning a list of "foo bar" strings, it
was returning a list of object references.  So I quickly hacked out a
more "correct"[3] return statement.  The final function looked like this:

sub getMacs {
  my ($host) = assertNumArgs(1,@_);
  my $ping = Net::Ping->new();

  my $iface = '((?:\w+(?:[0-9]+|:[0-9]+))|lo)'                          ;
  my $macRegex  = 'HWaddr\s([A-Fa-f0-9]{1,2}(?::[A-Fa-f0-9]{1,2}){5})'  ;

  # check if we can get to the host to check it's Mac addresses
  if (!$ping->ping($host)) {
    $log->error("Can not ping $host to obtain MAC Addresses");
    return;
  }

  # find out what interfaces it has from /proc
  my $interfaces
    = assertRemoteCommand($host, "grep : /proc/net/dev", "-l root");

  return map { chomp($_->{stdout});
               $_->{stdout} =~ s/$iface.*$macRegex/$1 $2/g;
               "$1 $2";
             } map { assertRemoteCommand($host,
                                         "/sbin/ifconfig $_ | grep HW",
                                         "-l root") }
                   grep { !/^lo$/ && s/^\s+// && s/:.*//g }
                     split(/\n/, $interfaces->{stdout});
}

And there you have it. 'return map {...} map {...} grep {...}
split(...);' Short, concise, no need for a bunch of temp variables
that only get used once and thrown away.  I like it.  Others, well,
not so much :)

One guy IM'ed me back (on the dev channel where this conversation
took place) and simply stated "I'm stunned!".  My friend the
perl/lisp hacker replied, "Wow.  That's a bit much, even for me!", and
still someone else replied, "Can you *do* that?!  I wouldn't expect
all that to work!"

I hope someone gets as much amusement out of this as I have :) Don't
try this at home folks!
-- 
Seeya,
Paul

[1] The only non-standard constructs in this code are the calls to
    assertRemoteCommand, which is a function which does what it says,
    or dies trying and returns an object with all the information
    you'd need to know about the command executed.  The comment
    structure above the function is also part of our standard style
    and is referred to as 'pdoc'.  It's something locally developed to
    mimic JavaDoc and does a fairly good job at it.

[2] On vacation, he once found himself with the recently released 2nd
    edition of the "Wizard Book", a laptop, but no net connection or
    scheme interpreter.  But since he had emacs and perl, he figured
    he could fake it and wrote his own scheme interpreter in perl :)

[3] It depends upon what your definition of "correct" is in this case :)
_______________________________________________
gnhlug-discuss mailing list
gnhlug-discuss@mail.gnhlug.org
http://mail.gnhlug.org/mailman/listinfo/gnhlug-discuss

Reply via email to