Evaluate coding and style

Ethan Furman ethan at stoneleaf.us
Thu Sep 24 23:12:18 CEST 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