Basic/beginner interactive program in Python

Question

I bring this code review plea over from suggestions from SO. I've made the suggested edits from that thread, except I kept one sys.exit() call because I was having trouble exiting otherwise. Overall, the code works as desired, and I'm looking for advice on how I can make it even cleaner.

import time
import sys
done = "I'm tired of you. Goodbye."
rainedOut = "Sorry, rain foiled your plans :("
dontUnderstand = "I'm sorry, I don't understand."
def good_weather():
 """Imagine a world where every 5 seconds it rains (good_weather = False),
 then is sunny again (good_weather = True). This function should return
 whether good_weather is True or False at the time it's called.
 """
 return (time.time() % 10) <= 5
def start():
 entries = 0
 while entries < 4:
 choice = raw_input("Hello! What do you want to do right now? Options: 1) Sleep, 2) Work, 3) Enjoy the great outdoors: ")
 if choice == "1":
 print "We are such stuff as dreams are made on, and our little life is rounded with a sleep. - Shakespeare, The Tempest"
 elif choice == "2":
 work()
 elif choice == "3":
 outdoors()
 else:
 print dontUnderstand
 entries += 1
 print done
def work():
 entries = 0
 entries2 = 0
 while entries < 4:
 choice = raw_input("Would you prefer sedentary office work or manual labor in the elements?: ")
 if "office" in choice:
 print "The brain is a wonderful organ; it starts working the moment you get up in the morning and does not stop until you get into the office. -Robert Frost"
 start()
 elif "manual" in choice:
 sunny = good_weather()
 if sunny == True:
 print "A hand that's dirty with honest labor is fit to shake with any neighbor. -Proverb"
 start()
 else:
 if entries2 < 3:
 print rainedOut
 entries2 += 1
 continue #restart loop
 else:
 print "You should probably just give up now."
 start()
 else:
 print dontUnderstand
 entries += 1
 print done
 sys.exit()
def outdoors():
 sunny = good_weather()
 if sunny == True:
 print "Adopt the pose of nature; her secret is patience. -Ralph Waldo Emerson"
 start()
 else:
 print rainedOut
 start()
start()

Answer 1

Ok here we go.

Style

You should get acquainted with the official python coding style, which is PEP8. I notice especially naming violations with your global variables, which should follow the rules of functions: lowercase with underscore separation.

If-elsing

Perhaps it's just me, but I find heavily usage of if/elif/else messy.

Python does not support the typical switchstatement that could fit here, but there is an idiom that resembles it in python by using a dictionary of functions.

for example:

options = {1 : sleep,
 2 : work,
 3 : outdoors,
}
options[entries]()

this will require you to create an additional sleep function that can be executed to print the text We are such stuff as dreams are made on

This technique however lacks the break system that is available in switch statements, but you don't seem to need that in your code.

Additionally this scales really easily, all you got to do is add a new method, nicely encapsulated, and add it to the dictionary.

Multiple end points

Your code seems to have multiple end points. At the end of your start() function you print done. But also in your work() function you print it and terminate the program manually. It is cleaner to have a smooth main function, with multiple options (may it be function calls), when the function calls return, you simply print done only there and the program stops.

Main function

It is a good habit to use a main function, perhaps even better to rename your start to main and use the following construct:

def main():
 # Your main code
if __name__ == "__main__":
 main()

Goodluck!

Answer 2

Hello the style of your code seems very good. You should limit your lines to a maximum to 79 as per PEP8. But otherwise you have a very good code style.

Also Python has the following cases for variables, etc.

• Classes: CammelCase.
• Constants: UPPER_SNAKE_CASE.
• Other, mostly: snake_case.

---

Your code seems to be repetitive in some cases, and very hard coded.

We should first aim to remove repetition, and remove the static code from the dynamic code.

start could become very repetitive very fast. It uses a 'case' statement, and should use a dict. This is good for later when you have more functions like this.

#Python2/3 support.
try:
 input_ = raw_input
except NameError:
 input_ = input
def choice(choice_hint, options, max_trys=4):
 def call():
 entries = 0
 while entries < max_trys:
 choice = raw_input(choice_hint)
 if choice in options:
 if options[choice]():
 break
 else:
 print(dontUnderstand)
 entries += 1
 return call

If your program didn't use good_weather() then you would only need two functions.
You need to pass functions, not strings. But it's simple.

I removed print done, as you can do that at the end of the program. I added the break, so that you don't have to call sys.exit.

Usage

def delayed_output(output, status=True):
 def call():
 print(output)
 return status
 return call
choice(
 "Hello! What do you want to do right now? Options: 1) Sleep, 2)"
 "Work, 3) Enjoy the great outdoors: ",
 {
 "1": delayed_output(
 "We are such stuff as dreams are made on, and our little life "
 "is rounded with a sleep. - Shakespeare, The Tempest",
 False),
 "2": work,
 "3": outdoors
 })()
print done

---

work is like start, where outdoors is like sleep.

The changes I would make to outdoors is the removal of start(). You call it on each branch. And you could do nothing and go back to start.
So we will return if we want to break or not.

def function_output(output, function, status=True):
 def call():
 try:
 print(output[function()])
 except KeyError:
 print('Error in `function_output`. Unhandled output.')
 return status
 return call

---

To improve work, I would use the above functions.

Also True is a singleton. when comparing to singletons use is. And you should not compare to True or False.

# Bad
>>> True == True
True
>>> False != True
True
# Ok for other singletons
>>> True is True
True
>>> False is not True
True
# Good
>>> True
True
>>> not False
True

Also passing the argument entries2 is annoying to implement. You would have to make every function take *args. Something I don't think beginners should know or use.
Also I wouldn't be able to just use the above functions.

---

To use everything, you would need to change the initial choice.

choice(
 "Hello! What do you want to do right now? Options: 1) Sleep, 2)"
 "Work, 3) Enjoy the great outdoors: ",
 {
 "1": delayed_output(
 "We are such stuff as dreams are made on, and our little life "
 "is rounded with a sleep. - Shakespeare, The Tempest",
 False),
 "2": choice(
 "Would you prefer sedentary office work or manual labor in "
 "the elements?: ",
 {
 "office": delayed_output(
 "The brain is a wonderful organ; it starts working "
 "the moment you get up in the morning and does not "
 "stop until you get into the office. -Robert Frost"
 ),
 "manual": function_output(
 {
 True: "A hand that's dirty with honest labor is fit "
 "to shake with any neighbor. -Proverb",
 False: rainedOut
 },
 good_weather)
 }),
 "3": function_output(
 {
 True: "Adopt the pose of nature; her secret is patience. "
 "-Ralph Waldo Emerson",
 False: rainedOut
 },
 good_weather)
 })()

---

This way we have no duplicate logic.
We have all the strings close together.
We can remove the global strings, and pass them through the initial function.

We could have the three functions in a local name-space. This would have the advantage of having nothing globally defined.

If you wanted to go a step further, than you could have the input as a JSON object. This would have the advantage of storing all the static information in a different file.

You could start the program like this:

def main():
 with open('my_file.json', 'r') as file_:
 data = json.load(file_)
 # Convert to runnable code, and run it.
 json_to_python_load(data)()

---

Pros and cons

I changed how your program worked. Quite a lot.

Pros

• Little to no duplicated logic.
• Splits static and dynamic code.
• Very little code.
• Increased maintainability.
• Reduce work to add more things.
• Useable on Python2 and Python3.
  If you dislike it then it is easy to remove.

Cons

• Not as changeable, we had to remove some of the outdoors logic.
  It wasn't really needed, as you were going to quit the next loop anyway.

• Can be hard to implement additional features.
  And if you do problems are likely to arise, unless you make it an optional feature.

• A near re-write of your code.

Answer 3

Diagnosis

You've recognized that sys.exit() is the symptom of a problem — namely, that you are misusing functions as if they were goto labels. Another symptom of this abuse is the excessively deep calls stacks: run this program for a while, then hit Ctrl C to see the stack trace. (If the call stack gets too deep, you'll end up crashing with a stack overflow.)

Suggested solution

@JoeWallis has written an excellent solution. The solution that I am presenting below is a less drastic rewrite that still addresses the root of your problem.

Note that I've renamed start() to main(). "Start" is a name that encourages you to treat it as a goto label.

The key observation is this: the goal of the main loop of the program is to display the main menu, generate a quote, then display the main menu again. Therefore, let's make it the main loop's responsibility to print the quote. The supplementary functions merely return either the quote to be displayed, or None to indicate a fatal exasperation.

import time
RAINED_OUT = "Sorry, rain foiled your plans :("
DONT_UNDERSTAND = "I'm sorry, I don't understand."
def is_sunny():
 """Imagine a world where every 5 seconds it rains (good_weather = False),
 then is sunny again (good_weather = True). This function should return
 whether good_weather is True or False at the time it's called.
 """
 return (time.time() % 10) <= 5
def sleep():
 return "We are such stuff as dreams are made on, and our little life is rounded with a sleep. - Shakespeare, The Tempest"
def work():
 bad_entries = 0
 rainy_days = 0
 while bad_entries < 4:
 choice = raw_input("Would you prefer sedentary office work or manual labor in the elements?: ")
 if "office" in choice:
 return "The brain is a wonderful organ; it starts working the moment you get up in the morning and does not stop until you get into the office. -Robert Frost"
 elif "manual" in choice:
 if is_sunny():
 return "A hand that's dirty with honest labor is fit to shake with any neighbor. -Proverb"
 elif rainy_days < 3:
 print RAINED_OUT
 rainy_days += 1
 else:
 return "You should probably just give up now."
 else:
 print DONT_UNDERSTAND
 bad_entries += 1
def outdoors():
 if is_sunny():
 return "Adopt the pose of nature; her secret is patience. -Ralph Waldo Emerson"
 else:
 return RAINED_OUT
def main():
 submenus = {
 "1": sleep,
 "2": work,
 "3": outdoors,
 }
 bad_entries = 0
 while bad_entries < 4:
 choice = raw_input("Hello! What do you want to do right now? Options: 1) Sleep, 2) Work, 3) Enjoy the great outdoors: ")
 submenu = submenus.get(choice, None)
 if submenu is not None:
 quote = submenu()
 if quote is None:
 break
 print quote
 bad_entries = 0
 else:
 print DONT_UNDERSTAND
 bad_entries += 1
 print "I'm tired of you. Goodbye."
main()

Additional observations

Use ALL_CAPS for constants.

entries and entries2 should have more descriptive names.

It is customary to place the "main" method at the end of the program.