Duncan Booth wrote:
> Just some minor points without changing the basis of what you have done
> here:
>
> Don't bother with 'readlines', file objects are directly iterable.
> Why are you calling both lstrip and rstrip? The strip method strips
> whitespace from both ends for you.
>
> It is usually a good idea with code like this to limit the split method to
> a single split in case there is more than one colon on the line: i.e.
> x.split(':',1)
>
> When you have a sequence whose elements are sequences with two elements
> (which is what you have here), you can construct a dict directly from the
> sequence.
>
> But why do you construct a dict from that input data simply to throw it
> away? If you only want 1 domain from the file just pick it out of the list.
> If you want to do multiple lookups build the dict once and keep it around.
>
> So something like the following (untested code):
>
> from __future__ import with_statement
>
> def loaddomainowners(domain):
> with open('/etc/virtual/domainowners','r') as infile:
> pairs = [ line.split(':',1) for line in infile if ':' in line ]
> pairs = [ (domain.strip(), owner.strip())
> for (domain,owner) in pairs ]
> return dict(lines)
>
> DOMAINOWNERS = loaddomainowners()
>
> def lookupdmo(domain):
> return DOMAINOWNERS[domain]
Using two list comprehensions mean you construct two lists, which sucks
if it's a large file.
Also, you could pass the list comprehension (or better yet a generator
expression) directly to dict() without saving it to a variable:
with open('/etc/virtual/domainowners','r') as fh:
return dict(line.strip().split(':', 1) for line in fh)
(Argh, that doesn't .strip() the key and value, which means it won't
work, but it's so simple and elegant and I'm tired enough that I'm not
going to add that. :-P Just use another genexp. Makes for a line
complicated enough that it could be turned into a for loop, though.)
--
--
http://mail.python.org/mailman/listinfo/python-list
