Evaluate my first python script, please

[Jonathan Gardner]

On Thu, Mar 4, 2010 at 10:39 AM, Pete Emerson <pemerson at gmail.com> wrote:
>
> #!/usr/bin/python
>

More common:

#!/usr/bin/env python

> import sys, fileinput, re, os
>
> filename = '/etc/hosts'
>
> hosts = []
>
> for line in open(filename, 'r'):
>        match = re.search('\d+\.\d+\.\d+\.\d+\s+(\S+)', line)
>        if match is None or re.search('^(?:float|localhost)\.', line):
> continue
>        hostname = match.group(1)

In Python, we use REs as a last resort. See "dir("")" or "help("")"
for why we don't use REs if we can avoid them.

The above is better represented with:

try:
  ipaddr, hostnames = line.split(None, 1)
except IndexError:
  continue

if line.find('float') >=0 or line.find('localhost') >= 0:
  continue


>        count = 0
>        for arg in sys.argv[1:]:
>                for section in hostname.split('.'):
>                        if section == arg:
>                                count = count + 1
>                                break
>        if count == len(sys.argv) - 1:
>                hosts.append(hostname)
>

You can use the "else" in a "for". as well.

for arg in sys.argv[1:]:
         for section in hostname.split('.'):
                 if section == arg:
                         break
else: # Run only if for completes naturally
          hosts.append(hostname)


It may be clearer to do set arithmetic as well.

> if len(hosts) == 1:
>        os.system("ssh -A " + hosts[0])

You probably want one of the os.exec* methods instead, since you
aren't going to do anything after this.

> else:
>        print '\n'.join(hosts)

Rather, the idiom is usually:

for host in hosts:
  print host

-- 
Jonathan Gardner
jgardner at jonathangardner.net