On Mon, 21 Jun 2010, Radek Vykydal wrote:
Chris Lumens wrote:
-# Get the IP address for a network device.
-def getIPAddress(dev):
+# Get the IP addresses for a network device.
+# returns dict of lists with keys 'ipv4' and 'ipv6'
+def getIPAddresses(dev):
if dev == '' or dev is None:
return None
You return a dict of two lists, but in the two places getIPAddresses is
called, you only use the concatenation of those two lists. It looks to
me like it makes more sense to always return a single list. We can
always figure out if an address is IPv6 or not later, should we care.
In one of the places, the order is important and the caller
code seems more readable to me with getIPAddresses returning
the dict because suggested list would require to detect type and sort
addresses in the list (I don't like the idea of having another
function/code doing this), or it would have to rely on assumption
of order addresses returned in the list.
How about having a parameter version='ipv4' to return ipv4 addresses
only, 'ipv6' to return ipv6 addresses and None to return list as
you suggested?
The caller code would be then:
ips = (isys.getIPAddresses(devname, version='ipv4') +
isys.getIPAddresses(devname, version='ipv6'))
... of course we could rely on implicit order ipv4, ipv6, but I like
to be more explicit for code reader.
A version parameter seems fine, but I would prefer to see it as an int rather
than a string:
version=4
version=6
With the default as version=None.
--
David Cantrell <dcantrell@xxxxxxxxxx>
Red Hat / Honolulu, HI
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
