Evaluate coding and style
Ethan Furman
ethan at stoneleaf.us
Thu Sep 24 17:12:18 EDT 2009
Brown, Rodrick wrote:
> I recently started playing with Python about 3 days now (Ex Perl guy) and wanted some input on style and structure of what I'm doing before I really start picking up some bad habits here is a simple test tool I wrote to validate home dirs on my system.
>
> Please evaluate and let me know what could have been done better. Once again this is really my first time using python.
All in all it looks pretty good. Some minor enhancements...
>
>
> $ ./homedir_exists.py root mqm pcap
> root successful!
> Directory: /var/arpwatch not found!
^^^^ false negative?
> pcap successful!
> mqm successful!
>
>
> $ cat homedir_exists.py
> #!/usr/bin/env python
>
> import sys, os
> from re import match
>
> userlist = []
> filename = '/etc/passwd'
>
> for user in sys.argv[1:]:
> userlist.append(user)
since sys.argv is already a list, you can do
userlist = sys.argv[1:]
> try:
> fh = open(filename)
> except IOError:
> print "No such filename: %s" % (filename)
>
> def checkDir(username):
> data = fh.readlines()
> for line in data:
> for user in username:
> if match(user,line):
> s = line.split(':')
> if not os.path.isdir(s[5]):
> print "Directory: %s not found!" % (s[5])
> print s[0] + " successful!"
>
> checkDir(userlist)
passwd has a well defined layout... instead of using re, I would split
each line into it's component fields, then just match the username
fields against the usernames passed in... this also avoids the false
negative shown in your example, and gets rid of one unnecessary loop.
for line in data:
fields = line.split(':')
user = fields[0]
path = fields[5]
if user in userlist:
if os.path.isdir(path):
print "%s successfull!" % user
else:
print "Directory %s for user %s not found!" % (path, user)
Cheers!
~Ethan~
More information about the Python-list
mailing list