Code Review

Philipp Hagemeister phihag at phihag.de
Sat Sep 17 13:19:01 CEST 2011


Instead of comments, you can often use docstrings
(http://www.python.org/dev/peps/pep-0257/ ):

This is hard to read due to the indentation, and cannot be accessed
programmatically:

#Update the GUI
    def update_gui(self, new_word):

Instead, use this:

    def update_gui(self, new_word):
        "Update the GUI."

Now, you can use help(Message) to get information about the method.
You'll notice "Update the GUI." is not helpfull at all for a method
called update_gui. Comments (and docstrings) that reproduce the method
name are not useful.

A couple of minor things:

* If you delete code, delete it, don't comment it out.
* Use newlines between two methods. Compare
def a(self):
  pass
def b(self):
  pass
def c(self):
  pass

with

def a(self):
  pass

def b(self):
  pass

def c(self):
  pass

The latter looks neat and not nearly as crammed as the former.
* Don't use newlines where they shouldn't be, for example in
                if val == 0:

                      label.unbind('<Button-1>')
* Even if it's just the comments, typos make a very bad impression of a
coder and the code. I'd automatically assume lots of bugs and untested
code when I see more than the occasional typo.
* GUI programming is fun, but does not lend itself to structured
programming and good practices. You should never use global. Instead,
have an object encapsulating the state and pass that object to the
method itself or its object.
* Don't commit .pyc files, they're totally useless. Since python 2.6,
you can add the following in your .bashrc to make python not create them:

export "PYTHONDONTWRITEBYTECODE=dont"

In git, you can add the following in your project's .gitignore or
~/.gitignore_global:

*.pyc

[More on .gitignore: http://help.github.com/ignore-files/ ]
* Otherwise, the code looks pretty good for a beginner. You may,
however, want to replace

def word_not_found(word):
      if word in searchedwordset:
        return 0
      else:
        return 1

with just:

def word_not_found(word):
  return word not in searchedwordset

(or just skip this method and write word not in searchedwordset).

Cheers,

Philipp



Emeka wrote:
> Hello All,
> 
> While learning Python I put together another Text Twist. I would want
> somebody to go through it and comment.
> https://github.com/janus/Text-Twist
> 
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: OpenPGP digital signature
URL: <http://mail.python.org/pipermail/python-list/attachments/20110917/ce8c1255/attachment.sig>


More information about the Python-list mailing list