Saw this on r/python

[rcludwick]

A few good comments were on there, just wanted to add a few more here.

I saw this line and it gave me shivers:

if revisedMatches == [] or revisedMatches == None:
    return None

That's a statement you'd see in Java or C. It is not something I like to see in Python.

Functions should strive to return one type and one type only. In python None is it's own type. In larger projects you are guaranteed to have the following scenario:

Moe, Larry, and Curly each maintain different parts of the codebase and Moe likes to return None sometimes.

def moe(a):
    if len(a):
        return len(a)
    return None

def larry(a):
    b = moe(a)
    # ... lots of code here ...
    return b

def curly(a):
    c = larry(a)
    # ... lots of code here ...
    c = c + 1 #TypeError   How did c become None?

So the problem is that moe() returns an int or None, which is two different types in python. larry() passes it blindly upwards to curly() which assumes that he's getting an int. This causes a TypeError when curly finally gets around to using it.

Backtracing to see who set your value to None is a bit like watching a Three Stooges movie as Curly has to go through Larry to find the source of the error, in this case Moe.

Compare that with the following code where Moe throws an exception:

class MoeError(Exception):
    pass

def moe(a):
    if len(a):
        return len(a)
    raise MoeError('Zoink!')

def larry(a):
    b = moe(a)
    # ... lots of code here ... 
    return b

def curly(a):
    try:
        c = larry(a) #MyError Received on this line.
        # ... lots of code here ...
        c = c + 1 #c is always an int now
    except MoeError:  #Exception block jumps here
        #do something here
        #'raise' to repropagate error

Now the MoeError can be handled with a try/except block if needed in curly() or just simply propagated up if it makes sense to do that. Curly is notified about it as soon as Moe throws it.

It is okay to return None In some circumstances where it would lead to uglier code. For instance the dictionary's get() method does this as an alternative to the standard method for accessing a dictionary key.

y = { 1 : 2 , 3 : 4 }
if len(y) > 1 and y.get(0) is not None:
   raise MyError('Dictionary needs a 0 key')

Also, matchWord() should be optimized since the list comprehension will run through the entire list every time it's run when you only need the first.

for key in dictionary:
    if dictionary[key] == word:
        return k
return 0

Or even better, remove matchWord() and simply reverse the dictionary so you get O(ln x) performance on dictionary lookups rather than O(x). (See below)

Finally, note this code:

# Open the dictionary/wordlist file 
wordList = open('wordlist.txt', 'r')
# Strip/remove the whitespace or \n at the end of each word, convert to lower case
words = [( line.lower() ).strip() for line in wordList]
# Put them all into a Python dict (hash table implementation)
dictionary = dict(zip(range(0,len(words)), words))

So words and range() are allocated in memory first which takes time to loop through them.

You can replace a list comprehension here with a generator comprehension like so:

# Open the dictionary/wordlist file
wordList = open('wordlist.txt', 'r')
# Strip/remove the whitespace or \n at the end of each word, convert to lower case
# Parentheses makes a generator comprehension instead of a list comprehension.
words = ( line.lower().strip() for line in wordList )
# Put them all into a Python dict (hash table implementation)
dictionary = dict(enumerate(words))

To reverse the dictionary to get the performance gain mentioned above use a dictionary comprehension:

dictionary = { y : x for x, y in enumerate(words) }

[gtdoubledouble]

Thank you for your suggestions. Its actually a little out of my knowledge bounds to digest right now but I'll implement the changes.