[Tutor] suggestions to improve design

[Prasad, Ramit]

> I've written a script to use within XChat (IRC client) and everything is
> working without issue. The script is a little tool to give an emote
> command and a handful of general pre written actions.
> 
> As it is now, the script works. But I've got maybe a dozen emotes in
> there, using if/elif loops to check against a list of emotes and
> format a string for display in the channel.
> 
> But, I also have another dictionary containing duplicate values with
> text placeholders where a %s would be substituted.
> 
> I feel that, as the list of available emotes grows, the script will
> suffer from a poor design. I have thought about pickling or using a file
> to store the dict or loops, but that only seems like a bandage and
> there may be a performance issue with potentially hundreds of loops to
> check through.
> 
> Ok, so I have posted my script at
> http://www.baked-potato.us/files/2ndthat.py
> 
> My question is, would anybody please take a look and offer suggestions
> on what I may do to improve this script?

Unless the script gets a *very* large list of emotes or you are on
an older machine I doubt that you will really see any performance
issues. That being said, my comments are below and other tutors
can correct anything I missed or said incorrectly. I like the 
presence of your documentation, kudos!

Why use a class for storing the 'tions' and dict directly from
the module?

tions = { [snip] }

I also think get_emote should not be passed the entire argv, 
it should be passed only the arguments it needs so I would change
the call to

def emote(argv):
    emote = argv[1]
    if emote in tions: # look at dictionary, no need to pull keys manually      
        emstring = get_emote(emote, argv[2:]) # pass only arguments
        
        # print a generic message to the channel displaying the emote string
        #xchat.emit_print("Generic Message", nick, emstring)
        xchat.command("me %s" % (emstring))

def get_emote( e, args ):
    if e in tions:
        if '%s' in tions[e]:
            return tions[e] % tuple(args) 
            # will error here if not the correct number of variables
            # in args; might want to catch and then return an error string
        else
            return tions[e]
    else:
        return 'Not an understood emote'


def list_em():
    print "There are %s emotes available." % (len(tions))
    print "Usage: /em \002<$emote>\002 - user supplied values are shown as \002<$bold>\002.\n"
    
    for key,value in tions.iteritems(): 
        print "Emote: %s - %s" % (key, value)


As a general note, you should use docstrings instead of comments 
above the function.

CHANGE

# show function - displays a single requested emote 
def show(s):    

TO 

def show(s):
    'displays a single requested emote' # single line docstring

OR

def show(s):
   ''' multi
       line 
       docstring
   '''


Ramit


Ramit Prasad | JPMorgan Chase Investment Bank | Currencies Technology
712 Main Street | Houston, TX 77002
work phone: 713 - 216 - 5423

--
This email is confidential and subject to important disclaimers and
conditions including on offers for the purchase or sale of
securities, accuracy and completeness of information, viruses,
confidentiality, legal privilege, and legal entity disclaimers,
available at http://www.jpmorgan.com/pages/disclosures/email.