5
\$\begingroup\$

How could I possibly shorten / clean this up? I suppose mainly clean up the loop at the start asking whether they would like to scramble the code.

def userAnswer(letters):
    print("Can you make a word from these letters? "+str(letters)+" :")
    x = input("Would you like to scrample the letters? Enter 1 to scramble or enter to guess :")
    while x == '1':
        print(''.join(random.sample(letters,len(letters))))
        x = input("Would you like to scrample the letters? Enter 1 to scramble or enter to guess :")
    word = input("What is your guess? :")
    word = word.lower()              
    if checkSubset(word, letters) == True and checkWord(word) == True:
        print("Yes! You can make a word from those letters!")
    else:
        print("Sorry, you cannot make that word from those letters")

userAnswer("agrsuteod")
\$\endgroup\$
  • \$\begingroup\$ What version of python are you using? \$\endgroup\$ – Ngenator Mar 29 '13 at 18:21
  • \$\begingroup\$ Some of your print statements have typos: "scrample" should be "scramble". \$\endgroup\$ – Jamal May 26 '14 at 17:58
6
\$\begingroup\$

Before presenting my version, I would like to make a couple of comments:

  • Use descriptive names. The name userAnswer gives me an impression of just getting the user's answer, nothing else. I like to suggest using names such as startScrambleGame, runScrambleGame or the like.
  • Avoid 1-letter names such as x -- it does not tell you much.
  • I don't know which version of Python you are using, but mine is 2.7 and input() gave me troubles: it thinks my answer is the name of a Python variable or command. I suggest using raw_input() instead.
  • Your code calls input() 3 times. I suggest calling raw_input() only once in a loop. See code below.
  • The if checkSubset()... logic should be the same, even if you drop == True.

Here is my version of userAnswer, which I call startScrambleGame:

def startScrambleGame(letters):
    print("Can you make a word from these letters: {}?".format(letters))
    while True:
        answer = raw_input('Enter 1 to scramble, or type a word to guess: ')
        if answer != '1':
            break
        print(''.join(random.sample(letters, len(letters))))

    word = answer.lower()              
    if checkSubset(word, letters) and checkWord(word):
        print("Yes! You can make a word from those letters!")
    else:
        print("Sorry, you cannot make that word from those letters")

startScrambleGame("agrsuteod")
\$\endgroup\$
  • 3
    \$\begingroup\$ In Python 3, raw_input() has been replaced with input(). \$\endgroup\$ – Adam Mar 29 '13 at 23:11
  • 2
    \$\begingroup\$ OK. Sorry for living in stone age :-) \$\endgroup\$ – Hai Vu Mar 29 '13 at 23:17
  • 1
    \$\begingroup\$ and the code uses print() as a function; the OP is almost certainly using Python 3. \$\endgroup\$ – Martijn Pieters May 26 '14 at 18:09
6
\$\begingroup\$

In Python there is an agreed-upon standard on how the code should look: PEP 8 -- Style Guide for Python Code. The standard is to use lowercase names with underscores.

For shuffling a list, there is a function in random aptly called random.shuffle. Use .format to interpolate values in string. Do not repeat yourself and just ask the thing once; one can combine these inputs together.

Do not needlessly compare against True using ==; just use the implied boolean value of the expression (and if you need to be explicit then use is instead of ==). Store the letters in a list for easier modification. Also there is no need to write check_subset as a function, if you really want a set operation (that is, the same letter can occur multiple times in the word, even if only once in the original).

Thus we get:

def ask_user(letter_word):
    letters = list(letter_word)
    while True:
        print("Can you make a word from these letters: {}?"
            .format(''.join(letters)))

        choice = input("Enter your guess or 1 to scramble: ")

        if choice == '1':
            random.shuffle(letters)
        else:
            break  # break out of the while loop, a guess was entered

    word = choice.lower()              
    if set(word).issubset(letters) and check_word(word):
        print("Yes! You can make a word from those letters!")
    else:
        print("Sorry, you cannot make that word from those letters")

ask_user("agrsuteod")
\$\endgroup\$
  • \$\begingroup\$ The Can you make a word ... introduction is displayed only once in the OP. \$\endgroup\$ – David Harkness May 26 '14 at 18:26
  • \$\begingroup\$ Yes, I did coalesce them together here, also I did change the input handling. \$\endgroup\$ – Antti Haapala May 26 '14 at 18:27
5
\$\begingroup\$

One minor thing of note is that

word = word.lower()

would be better served with

word = word.casefold()

Here are the docs.

Return a casefolded copy of the string. Casefolded strings may be used for caseless matching.

Casefolding is similar to lowercasing but more aggressive because it is intended to remove all case distinctions in a string.

\$\endgroup\$
  • \$\begingroup\$ +1 for mentioning casefold here, but again would be better is debatable. \$\endgroup\$ – Antti Haapala May 26 '14 at 18:32
3
\$\begingroup\$

For one thing, if checkSubset() and checkWord() return bools, you can leave out the == True.

\$\endgroup\$
2
\$\begingroup\$

Don't repeat yourself, like I have assigned the message to a variable msg so that if you need to change the message in future then, you don't have to edit at more than one place. Also, you don't need to compare for boolean values as if statement takes care of that.

def userAnswer(letters):
    print("Can you make a word from these letters? "+str(letters)+" :")
    msg = "Would you like to scrample the letters? Enter 1 to scramble or enter to guess :"
    x = input(msg)
    while x == '1':
        print(''.join(random.sample(letters,len(letters))))
        x = input(msg)
    word = input("What is your guess? :").lower()
    if checkSubset(word, letters) and checkWord(word):
        print("Yes! You can make a word from those letters!")
    else:
        print("Sorry, you cannot make that word from those letters")

userAnswer("agrsuteod")
\$\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.