Evaluate my first python script, please

Jean-Michel Pichavant jeanmichel at sequans.com
Fri Mar 5 05:44:57 EST 2010


Pete Emerson wrote:
> I've written my first python program, and would love suggestions for
> improvement.
>
> I'm a perl programmer and used a perl version of this program to guide
> me. So in that sense, the python is "perlesque"
>
> This script parses /etc/hosts for hostnames, and based on terms given
> on the command line (argv), either prints the list of hostnames that
> match all the criteria, or uses ssh to connect to the host if the
> number of matches is unique.
>
> I am looking for advice along the lines of "an easier way to do this"
> or "a more python way" (I'm sure that's asking for trouble!) or
> "people commonly do this instead" or "here's a slick trick" or "oh,
> interesting, here's my version to do the same thing".
>
> I am aware that there are performance improvements and error checking
> that could be made, such as making sure the file exists and is
> readable and precompiling the regular expressions and not calculating
> how many sys.argv arguments there are more than once. I'm not hyper
> concerned with performance or idiot proofing for this particular
> script.
>
> Thanks in advance.
>
> ########################################################
> #!/usr/bin/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)
> 	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)
>
> if len(hosts) == 1:
> 	os.system("ssh -A " + hosts[0])
> else:
> 	print '\n'.join(hosts)
>   
You've already been given good advices.
Unlike some of those, I don't think using regexp an issue in any way. 
Yes they are not readable, for sure, I 100% agree to that statement, but 
you can make them readable very easily.

# not readable
match = re.search('\d+\.\d+\.\d+\.\d+\s+(\S+)', line)


# readable
match = re.search('\d+\.\d+\.\d+\.\d+\s+(\S+)', line) # match 
'192.168.200.1   (foo123)'

I gladly understand ppl that are not willing to use regexp (or as last 
resort), but for a perl guy, that should not be an issue.

JM




More information about the Python-list mailing list