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 d