2
\$\begingroup\$

I am new to coding and am working through Learn Python The Hard Way. I wrote a simple text game as instructed. The logic appears to be sound. I played through it and didn't run into any dead ends. I am simply looking for some pointers on how to make my code more neat, more readable, more efficient etc. I would like to avoid picking up bad habits in the early stage of my learning.

from sys import exit
key = False
def safe():
 print "\tEnter three numbers to try the safe."
 combination = raw_input("> ")
 if combination == "10 21 15":
 print """
 Success! The safe opens. Inside, you
 find a key and some cash.
 """
 global key
 key = True
 room1Menu()
 elif combination != "10 21 15":
 print """
 That didn't seem to work.
 There must be a clue around
 somewhere.
 """
 room1Menu()
 else:
 dead("\tYou notice a strange substance on your fingers. You smell it and die instantly.")
def picture():
 print """ 
 You gaze at the picture, then inspect
 it with your hands. You tug and the
 edges and it opens to reveal an old
 fashioned wall safe. It has a dial
 with numbers that range from 0 to 50. 
 You remember from your high school locker 
 that these things require three numbers.
 Enter '1' to continue to look around the room.
 Enter '2' to try to open the safe.
 """
 action = raw_input("> ")
 if action == "1":
 room1Menu()
 elif action == "2":
 safe()
 else:
 dead("\tYou notice a strange substance on your fingers. You smell it and die instantly.")
def dresser():
 print """ 
 The drawers are empty and there
 are several useless objects on top.
 There is a folded slip of paper.
 You open it and it reads,
 "Great Scott! It's the future today!"
 """
 room1Menu()
def bed():
 print "\tThere is nothing of interest here."
 room1Menu()
def room1Menu():
 print """
 Enter '1' to investigate the dresser.
 Enter '2' to investigate the bed.
 Enter '3' to investigate the picture on the wall.
 Enter '4' to exit the room.
 """
 action = raw_input("> ")
 if action == "1":
 dresser()
 elif action == "2":
 bed()
 elif action == "3":
 picture()
 elif action == "4":
 hallway()
 else:
 dead("\tYou panic, have a heart attack and die.")
def room1():
 print """
 The room is sparse. There is 
 a dresser, a bed and a picture
 on the wall depicting a mysterious
 looking soldier.
 """
 room1Menu()
def hallway():
 print """
 Enter '1' to try the door to the room inside.
 Enter '2' to try the door that appears to lead outside.
 """
 action = raw_input("> ")
 if action == "1":
 print "\tYou open the door and enter."
 room1()
 elif action == "2" and key == False:
 print """ 
 You try the door, but it is
 locked from the outside. There
 may be a key around somewhere...
 """
 hallway()
 elif action == "2" and key == True:
 print """
 You try the key and it fits!
 You open the door to freedom
 and live out your days wasting
 time looking at memes on the
 internet.
 """
 exit(0)
 else: 
 dead("\tYou pace in circles until you dehydrate and die.")
def dead(why):
 print why, " Game Over."
 exit(0)
def start(): 
 print """ 
 You suddenly awake in a dark hallway.
 Your head hurts and you have no idea
 how you got here.
 Once your eyes adjust, you see that
 there are two doors. One looks like
 a door to another room, the other looks
 like a door that leads outside.
 """
 hallway()
start()
SuperBiasedMan
13.5k5 gold badges37 silver badges62 bronze badges
asked Oct 23, 2015 at 3:23
\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

Your flow is a bit off. You repeatedly call room1Menu at the end of each function. What you should do instead is have that function loop forever, until something actually ends the whole program. So you wrap all of room1Menu's code in a while True loop and then exit when the game is actually over. I would also take the menu code and put it in room1 instead, then you can just print the introduction once followed by the infinite loop. Something like this:

def room1():
 print """
 The room is sparse. There is 
 a dresser, a bed and a picture
 on the wall depicting a mysterious
 looking soldier.
 """
 while True:
 print """
 Enter '1' to investigate the dresser.
 Enter '2' to investigate the bed.
 Enter '3' to investigate the picture on the wall.
 Enter '4' to exit the room.
 """
 action = raw_input("> ")
 if action == "1":
 dresser()
 elif action == "2":
 bed()
 elif action == "3":
 picture()
 elif action == "4":
 hallway()
 else:
 dead("\tYou panic, have a heart attack and die.")

Next, you should sanitise input a bit. If someone enters "3 " it's a bit harsh to give them a heart attack. You can remove whitespace from either side of a string using strip(). I would chain it onto the raw_input call, and it's good practice to do this often.

 action = raw_input("> ").strip()

In your safe you end up testing first if combination is a specific value, then if it's not that specific value and then anything else? It's not possible to reach that third option. If combination is blank it will still catch the second value. If combination somehow didn't exist then you'd be getting errors, not reaching an else block. I suspect that you were trying to cover in case it somehow fell through both conditions, but it actually just can't happen so you can remove the redundant code and instead just have if combination == "10 21 15": and else:.

Instead of using key == False or key == True you should just test key. Since it's a boolean it has the same effect and looks neater.

In a couple places you call exit(0), but it actually defaults to zero so you don't need to explicitly include that value.

answered Oct 23, 2015 at 8:59
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Ah, thank you. The text I am using to help me through this uses an example with an infinite loop using the 'while True' statement. I was having a hard time understanding why this was beneficial. You just cleared it up for me. That redundancy should have been obvious to me and thanks for introducing me to strip(). That is handy. It was bugging me that there was a bunch of white space before those tabbed strings when they printed to the screen. \$\endgroup\$ Commented Oct 23, 2015 at 14:11
3
\$\begingroup\$
if combination == "10 21 15":
elif combination != "10 21 15":
else:

Those last two are the exact same conditions, if you want a working option, consider using the random library to select a choice: random.choice('live', 'die').


Instead of using globals (global key): use a Class structure instead:

class HallwayGame(self):
 def __init__(self):

Instead of an if and elif block: use an object instead:

 if action == "1":
 dresser()
 elif action == "2":
 bed()
 elif action == "3":
 picture()
 elif action == "4":
 hallway()
 else:
 dead("\tYou panic, have a heart attack and die.")

Consider using predefined variables instead of printing the results, e.g:

responses = {

Considering those changes along with a few more in the code, your code would look like the following:

from sys import exit
import random
class HallwayGame:
 def __init__(self):
 self.key = False
 self.responses = {
 'dead_finger_poison': "\tYou notice a strange substance on your fingers. You smell it and die instantly.",
 'dead_heart_attack': "\tYou panic, have a heart attack and die.",
 'dead_circle_dehydrate': "\tYou pace in circles until you dehydrate and die.",
 'intro': """ 
 You suddenly awake in a dark hallway.
 Your head hurts and you have no idea
 how you got here.
 Once your eyes adjust, you see that
 there are two doors. One looks like
 a door to another room, the other looks
 like a door that leads outside.
 """,
 'safe_intro': "\tEnter three numbers to try the safe.",
 'safe_success': """
 Success! The safe opens. Inside, you
 find a key and some cash.
 """,
 'safe_failure': """
 That didn't seem to work.
 There must be a clue around
 somewhere.
 """,
 'picture_intro': """ 
 You gaze at the picture, then inspect
 it with your hands. You tug and the
 edges and it opens to reveal an old
 fashioned wall safe. It has a dial
 with numbers that range from 0 to 50. 
 You remember from your high school locker 
 that these things require three numbers.
 Enter '1' to continue to look around the room.
 Enter '2' to try to open the safe.
 """,
 'dresser_intro': """ 
 The drawers are empty and there
 are several useless objects on top.
 There is a folded slip of paper.
 You open it and it reads,
 "Great Scott! It's the future today!"
 """,
 'bed': "\tThere is nothing of interest here.",
 'room_1': """
 The room is sparse. There is
 a dresser, a bed and a picture
 on the wall depicting a mysterious
 looking soldier.
 """,
 'room_1_menu': """
 Enter '1' to investigate the dresser.
 Enter '2' to investigate the bed.
 Enter '3' to investigate the picture on the wall.
 Enter '4' to exit the room.
 """,
 'hallway_menu': """
 Enter '1' to try the door to the room inside.
 Enter '2' to try the door that appears to lead outside.
 """,
 'hallway_inside_door': "\tYou open the door and enter.",
 'hallway_outside_door_locked': """ 
 You try the door, but it is
 locked from the outside. There
 may be a key around somewhere...
 """ ,
 'hallway_outside_door_unlocked': """
 You try the key and it fits!
 You open the door to freedom
 and live out your days wasting
 time looking at memes on the
 internet.
 """
 }
 def safe(self):
 print(self.responses['safe_intro'])
 combination = input("> ")
 if combination=="10 21 15":
 print(self.responses['safe_success'])
 self.key = True
 self.room1Menu()
 else:
 if random.choice('die', 'live')=='die':
 self.dead(self.responses['dead_finger_poison'])
 else:
 print(safe_failure)
 self.room1Menu()
 def picture(self):
 print(self.responses['picture_intro'])
 action = input("> ")
 if action=="1":
 self.room1Menu()
 elif action=="2":
 self.safe()
 else:
 self.dead(self.responses['dead_finger_poison'])
 def dresser(self):
 print(self.responses['dresser_intro'])
 self.room1Menu()
 def bed(self):
 print(self.responses['bed'])
 self.room1Menu()
 def room1Menu(self):
 print(self.responses['room_1_menu'])
 action = int(input("> "))
 actions = {
 1: self.dresser,
 2: self.bed,
 3: self.picture,
 4: self.hallway
 }
 if action in range(1,4):
 actions[action]()
 else:
 print(action)
 self.dead(self.responses['dead_heart_attack'])
 def room1(self):
 print(self.responses['room_1'])
 self.room1Menu()
 def hallway(self):
 print(self.responses['hallway_menu'])
 action = input("> ")
 if action=="1":
 print(self.responses['hallway_inside_door'])
 self.room1()
 elif action=="2":
 if self.key:
 print(self.responses['hallway_outside_door_unlocked'])
 exit(0)
 else:
 print(self.responses['hallway_outside_door_locked'])
 self.hallway()
 else: 
 self.dead(self.responses['dead_circle_dehydrate'])
 def dead(self, why):
 print(why, " Game Over.")
 exit(0)
 def start(self): 
 print(self.responses['intro'])
 self.hallway()
HG = HallwayGame()
HG.start()
answered Oct 23, 2015 at 4:42
\$\endgroup\$
1
  • \$\begingroup\$ Thank you very much for your reply. The redundant third option in my if statement should have been obvious to me. I have only been coding for a couple weeks, so I am not yet familiar with a lot of the concepts you introduced, but now I have some direction and know what to focus on. Thanks again. \$\endgroup\$ Commented Oct 23, 2015 at 13:54

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.