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()
\$\endgroup\$
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.

\$\endgroup\$
  • 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\$ – Fishcadet Oct 23 '15 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()
\$\endgroup\$
  • \$\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\$ – Fishcadet Oct 23 '15 at 13:54

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.