Please rate my scripts.

Michael Torrie torriem at gmail.com
Mon Apr 14 10:15:36 MDT 2008


Stuart Jansen wrote:
> The target audience is system administrators, possibly with no previous
> programming experience. I want the scripts to use the same algorithm as
> the shell script, be readable by a newbie, but also capture the flavor
> of each language. Because I consider myself a jack of all languages,
> master of none, I'd appreciate if you took a moment to check my
> translation to your favorite language and let me know if I've done
> anything offensive.

How "pythonic do you want your script?  Here are a couple of points:

- None of your scripts have any comments at all.  Shell scripting in
particular requires many comments, in my experience.
- Only ever compile your regex's once if you can help it.  Looks to me
like filter should be defined right at the top.
- for ease of maintainability, only define the "cmd" variable once at
the top, and then use the "%" operator in context to fill in the proper
variable (I'll attach code that demonstrates this)
- in Python, avoid comparisons to 0 in cases like this.  Just use the
"if not" thing.  Only because it's the "pythonic" way.  Granted in this
case it doesn't matter, but in other cases it should be avoided.  Like
comparison to Null, for example.
- In any language, including bash, I highly recommend using getopt to
provide simple command line help and rudimentary switch processing if
needed (which it's not in this case).  Right now, running the script
without any arguments immediately starts doing some work, which is
something that's a bit dangerous in my opinion (although we all do it!)

So here's some refined code for you:
#!/usr/bin/python

import os
from os.path import abspath, dirname, exists
import re
import sys

DIR  = abspath( dirname( dirname( __file__ ) ) )
DICT = abspath( DIR+'/doc/aspell.en.pws' )
cmd = "git diff --name-only '%s'"
filter = re.compile( '.*/.*\.xml$', re.M )
dir='master' # default directory to process

def spellcheck( file ):
    print "Spell checking %s" % file
    cmd = "aspell -H -p '%s' -c '%s'" % (DICT, file)
    os.system( cmd )

argv = sys.argv[1:]
argc = len( argv )

if not argc or (argc == 1 and not exists(argv[0])):
    if argc: dir=argv[0]

    files  = os.popen( cmd % dir ).read()

    for file in filter.findall( files ):
        spellcheck( file )
else:
    for file in sys.argv[1:]:
        spellcheck( file )




More information about the PLUG mailing list