5
\$\begingroup\$

I am a beginner trying to fetch a random line from a file and wondering if this code can be improved. Should I use try and catch in this case? Should I take a parameter for the function "quote"? How can I take several alternatives for searchText? (E.g. "hello", "hi" or "hey" but one is enough to return true.)

def getInputFromUser(inputText, verifier, error=None):
    """
    General function to get input from the user, repeating the question until verifier returns true
    """
    while True:
        userInput = input(inputText)
        if not verifier or verifier(userInput):
            return userInput
        elif error:
            print(error)

def quote():
    """
    Return a random line from a file if user enters the text "hello"
    """
    searchText = "hello"
    text = getInputFromUser("Enter a sentence with the words 'hello', 'hi' or 'hey': ", lambda inputText: inputText)

    if searchText in text:
        lineFetched = random.choice(open('myquotes.txt').readlines())
        print("My quote: ", lineFetched)
    else:
        quote()
\$\endgroup\$
  • 2
    \$\begingroup\$ "Enter a sentence with the word "hello"': " is broken — please fix. \$\endgroup\$ – 200_success Mar 5 '15 at 16:55
4
\$\begingroup\$

When working with files, you should wrap open calls in a with statement like this:

if search_text in text:
    with open('myquotes.txt') as fh:
        line_fetched = random.choice(fh.readlines())
    print("My quote: ", line_fetched)
else:
    quote()

If you want to make it possible to match any of multiple words like "hello", "hi", then you'd need a helper function, for example:

def text_contains_any_of(text, words):
    for term in words:
        if term in text:
            return True
    return False

And then change your code to use this:

words = ("hello", "hi")

if text_contains_any_of(text, words):
    with open('myquotes.txt') as fh:
        line_fetched = random.choice(fh.readlines())
    print("My quote: ", line_fetched)

In the above examples I renamed the variable and method names to follow PEP8.

\$\endgroup\$
  • \$\begingroup\$ @ janos Thank you! How should I assign multiple words to "words"? As a list or one string? \$\endgroup\$ – momo Mar 5 '15 at 16:21
  • \$\begingroup\$ A list or tuple. I added an example now using a tuple \$\endgroup\$ – Stop ongoing harm to Monica Mar 5 '15 at 16:23
1
\$\begingroup\$

You have defined a nice getInputFromUser() function that can loop until the desired input is received. Why not take proper advantage of it?

def quote():
    SEARCH_TEXT = "hello"
    get_input_from_user(("Enter a sentence with the word '%s': " % SEARCH_TEXT),
        lambda s: SEARCH_TEXT in s)
    with open('myquotes.txt') as f:
        print("My quote: ", random.choice(f.readlines()))
\$\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.