2
\$\begingroup\$

I am looking to build a list of words (three letters or more) that make up a large list of six-letter words. I was wondering if there was a cleaner or more efficient way to loop through the characters to build these subsets.

The top of this program (not posted) takes two .txt files - one containing the list of 6-letter words and one containing the Scrabble Dictionary (used to determine what is a real word).

The main core of my code is below. I loop through each character of the word, making sure indexes don't repeat and then build each word subset. I then sort the list on length and convert it into a string via .join to feed into another app I am writing for "subset guessing".

# Define Dictionary Lookup Function
def isDictionaryWord(word):
    if (word.lower().strip()) in listDictionary:
        return True
    else:
        return False


def createSubsets(word):
    print 'Starting Word Subset - ' + word
    starttime = datetime.datetime.now()
    tempOutput = []

    ## 1-Letter Word Loop
    for c in range(0, len(word)):

        ## 2-Letter Word Loop (skip)
        for d in range(0, len(word)):
            if (c != d):

                    ## 3-Letter Word Loop
                    for e in range(0, len(word)):
                        if (c != e) and (d != e):
                            threeLetterWord = word[c] + word[d] + word[e]
                            if isDictionaryWord(threeLetterWord) and (threeLetterWord not in tempOutput):
                                tempOutput.append(threeLetterWord)
                                #print threeLetterWord

                            ## 4-Letter Word Loop
                            for f in range(0, len(word)):
                                if (c != f) and (d != f) and (e != f):
                                    fourLetterWord = word[c] + word[d] + word[e] + word[f]
                                    if isDictionaryWord(fourLetterWord) and (fourLetterWord not in tempOutput):
                                        tempOutput.append(fourLetterWord)
                                        #print fourLetterWord

                                    ## 5-Letter Word Loop
                                    for g in range(0, len(word)):
                                        if (c != g) and (d != g) and (e != g) and (f != g):
                                            fiveLetterWord = word[c] + word[d] + word[e] + word[f] + word[g]
                                            if isDictionaryWord(fiveLetterWord) and (fiveLetterWord not in tempOutput):
                                                tempOutput.append(fiveLetterWord)
                                                #print fiveLetterWord

    endtime = datetime.datetime.now()


    ## Sort Array by Length for Game Purposes & Convert to String
    tempOutput.sort(key = len)
    tempOutputStr = word + ": " + ", ".join(tempOutput)

    print 'Total Time Taken (s) - ' + str((endtime - starttime).total_seconds())
    print 'Total Subset Words   - ' + str(len(tempOutput) - 1) + '\n'
    print 'Temp Output Array    - ' + str(tempOutput) + '\n'
    print 'Temp Output String   - ' + tempOutputStr
    print '========================================'

    # Print tempOutputStr
    fOut.write(tempOutputStr + '\n')



# Loop through all 6-letter words and create subsets
for word in listSix:
    createSubsets(word)
\$\endgroup\$
2
\$\begingroup\$

Here are some comments, in no particular order:

  • You can simplify the definition of isDictionaryWord():

    def isDictionaryWord(word):
        return word.lower().strip() in listDictionary
    

    although as it stands, that won’t work because listDictionary isn’t defined.

    Speaking of that function, this comment:

    # Define Dictionary Lookup Function
    

    tells me nothing I couldn’t learn from reading the next line. It’s superfluous and should be removed.

  • Have a read of PEP 8, particularly with regards to variable naming. Python uses snake_case for most variables, with CamelCase reserved for classes only.

  • Since you’re using Python 2.x (I can tell from the print statements), it’s more efficient to use xrange() instead of range() – this creates a memory-efficient iterator rather than creating a list, which can make your code much faster over large ranges.

  • You could tidy up some of your checks by making tempOutput a set instead of a list, then uniqueness is enforced by Python. Your innermost checks become more like:

    if is_dictionary_word(three_letter_word):
        temp_output.add(three_letter_word)
    

  • Rather than writing out the loops explicitly, I’d suggest looking at the itertools module. This contains some very fast functions for doing big loops. For example, your check for three-letter words reduces to the following:

    import itertools
    
    temp_output = []
    
    for tmp_word in itertools.permutations(word.lower(), r=3):
        if is_dictionary_word(tmp_word):
            temp_output.append(tmp_word)
    

    You can add similarly simple loops to get the four and five-letter words.

    This has a few other benefits:

    • This approach enforces uniqueness, so we can use a list (which preserves ordering) without needing to check if a word is already there.
    • The array is already sorted by length of the word, with the shortest words first.
    • By lowercasing the main word once, we can speed up our calls to is_dictionary_word by getting rid of the .lower() and .strip() calls.

  • There’s too much code in the createSubsets() function. That function includes:

    • Printing debug code about what it’s doing, and how long it took
    • Finding a list of subset words
    • Writing that list to a file

    It would be better if each of those tasks were handled by separate functions – this would allow you to reuse code more easily among different parts of your code.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.