[Tutor] suggestions to improve design

[Justin Straube]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/04/2012 11:28 AM, Prasad, Ramit wrote:
> 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

Thank you for your time, Ramit.

> can correct anything I missed or said incorrectly. I like the 
> presence of your documentation, kudos!

As a hobbyist, I can afford the time to document. I also find that
even with code I wrote, the documentation helps me understand better,
and find areas that need more work.

> Why use a class for storing the 'tions' and dict directly from the
> module?
> 
> tions = { [snip] }

When I started, I was unsure what else I would be putting in the class
Emo. After I got going, I was unsure about changing that, if the
design needed adjusting and it would be used further. Before I do to
much more for this, I am going to remove the class and just use the
dictionary, with a better name.

Thank you for your other ideas. Ill work over this and also clean up
my comments.

Justin


> 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

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.16 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJPzYq7AAoJEBXz28t2j4R5MT4H/2ILvS33kQmW5r8sNyeszoir
p5BNJOOEcEpc9Th41+12hXrP1Jir0COUur1HKqoom6Ufx07FuYPhiQ2MOC7W43EZ
I9m8/F0b/DkxLBBwg4CL+tfTAhIZ9R2/rfKxQ8GQSN4Yn29X6WGrP/uK9TRlHERq
ucb3tYPOp8b/zMujILVjndkqozupqNTrGZVBe8F8tphcnlv6ScZHjDRsaK898SbL
mcK/Ua45XVpaGpunIV8Um03hVf+UZwwvat348vOE8fgsOyywXaZc0oHsVr6G41os
zsQXR2/3CJA88ZVqJzRKmwdHJ2xuPsNWnrTuxplsHnsNWuHMHBHmnPj+HC9Gnsw=
=BPDI
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0x768F8479.asc
Type: application/pgp-keys
Size: 1744 bytes
Desc: not available
URL: <http://mail.python.org/pipermail/tutor/attachments/20120604/a033da7b/attachment-0001.key>