[Tutor] first program

[Danny Yoo]

On Sat, 6 Dec 2003, RoT wrote:

> I have coded my first non toy application since studying python for
> about 2 months now. I would love to hear any comments or suggestions
> regarding my coding style, methods, or suggestions of what I may need to
> learn or brush up on, or what I may be doing completely wrong
> (unpythonic).
>
> http://www.245t.com/spamkiller/SpamKiller.py

Hi Kris,


I'm reading through it now; very impressive so far!  I'll try giving
constructive criticism.


Small nitpick:  'retreive' should be 'retrieve'.  But I should be wary
about correcting the spelling of others.  There's some danger of the
kettle calling the pot black.  *grin*


In some places, the program uses the 'string' module.  For most purposes,
its use is deprecated because a lot of that functionality now lives within
each string as a string method.  But don't worry: it's fairly easy to fix
this.  For example:

    string.find(sub[message], word) != -1

can be translated as:

    sub[message].find(word) != -1


I see that you're using map() to do some string stripping.  I personally
like map().  *grin* Some folks may be more comfortable with list
comprehensions.  Concretely, an expression like:

    map(string.strip, ['   this ', '   is ', 'a    ', '   test'])

can be written as:

    [s.strip() for s in ['   this ', '   is ', 'a    ', '   test']]

Personally, I wouldn't change this in your program.  But it's good to know
about list comprehensions, just in case you see it in other people's
programs.



For the most part, your functions have pretty good names.  I want to focus
on some of the ones near the bottom as a style nitpick.

###
def addadd(lst): # add address
def rmadd(lst): # remove address
def pradd(lst): # print addresses
###

I'd bite the bullet and make the names just slightly longer, to lessen the
need for the comments.

###
def add_address(lst): # add address
def remove_address(lst): # remove address
def print_address(lst): # print addresses
###

I know, I'm being nitpicky!  But addadd(), rmadd(), and pradd() might be
potentially confusing because they can sound a lot like "add add", "remove
add", and "product add".  *grin*


The change-option function 'chopt()' can be simplified by using a
"dispatch" table technique.  Here's a quick example to demonstrate the
idea:

###
def add(x, y): return x + y
def sub(x, y): return x - y
dispatch_table = {'+' : add,
                  '-' : sub}
while 1:
    action = raw_input("choose action: ")
    if action in dispatch_table:
        print dispatch_table[action](1009, 2009)
###




getlist() is using the 'in' operator, but on a 1-tuple:

###
def getlist(): # get list of setup options
	lst = []
	while True:
		x = raw_input("\n>>> ")
		if x in ("",): continue
		if x in (".",): break
		else: lst.append(x)
	return lst
###


This works, but it's simpler to do the direct comparison:

###
def getlist(): # get list of setup options
	lst = []
	while True:
		x = raw_input("\n>>> ")
		if x == "": continue
		if x == ".": break
		else: lst.append(x)
	return lst
###

Then again, you may have been experimenting with allowing different
choices for ending or ignoring input.  But since you only have 1-tuples in
there now, it might be good to switch back to the direct comparisons.


Last comment for now: the last main() method might be a bit long; you may
want to decompose it as you have the rest of the program.


Anyway, I hope these comments help.  Good luck to you!