I've been working on a text-based Zork-style game project for the last couple of months to teach myself Python. I have barely any experience coding prior to this project, but thanks to Stack Exchange and youtube I've made decent progress.

About a week ago I posted a version of my code here to Code Review, and I've been able to work in most of the suggestions I was given. However I've been told that there may be some deeper issues in my code (inconsistencies in my code was described to me as "code smell").

import Rooms
import GameItems
world = {}

class Items:
    def __init__(self, name, info, weight):
        self.name = name
        self.info = info
        self.weight = weight

class DoorKeys(Items):
    def __init__(self, name, info, weight):
        super().__init__(name, info, weight)

class Weapon(Items):
    def __init__(self, name, info, damage, speed, weight):
        super().__init__(name, info, weight)
        self.damage = damage
        self.speed = speed


class Room:

    def __init__(self, name, description, exits, actions, roominv, roomkey, lock):
        self.name = name
        self.description = description
        self.exits = exits
        self.actions = actions
        self.roominv = roominv
        self.roomkey = roomkey
        self.lock = lock

Rooms.world['forest path']
Rooms.world['inside shack']

class Player:

    def __init__(self, name, health):
        self.name = name
        self.health = health

class Location:

    def __init__(self, room):
        self.room = Rooms.world[room]

    def travel(self, direction):
        if direction not in Location.room.exits.keys():

    def SetNrN(self, direction):
        NrN = self.room.exits[direction]
        print("moving to", NrN)

    def KeyCheck(self, NrN):
        if Rooms.world[NrN].lock and Rooms.world[NrN].roomkey not in bag.inventory:
            Rooms.world[NrN].lock = False

    def SetRoom(self, NrN):
        self.room = Rooms.world[NrN]

    def NoExit(self):
        print("You can't go that way!")

    def NoKey(self):
        print('The door is locked! You need the right key!')

    def RoomDesc(self):

class Bag:

    def __init__(self, inventory):
        self.inventory = inventory

    def addToInventory(self, key):
        del Location.room.roominv[key]

    def SearchRoom(self):
        if Location.room.roominv:
            for item in list(Location.room.roominv.keys()):
                print("you find a", item)
            print("You don't find anything")

    def NoneHere(self, key):
        print("You can't find a", key)

    def CheckTake(self):
        key = command.split()[1]
        if Location.room.roominv and key in Location.room.roominv:
            print('you take the', key)

    def CheckInv(self):
        for item in list(bag.inventory):
            print("Your bag contains:", item.name)

player = Player("Jeff", 100)
bag = Bag([])
Location = Location('introd')

command = '  '
while command != "":
    command = input('>>> ')
    if command in Location.room.exits:
    elif command == 'look':
    elif command == '':
        print('You have to say what it is you want to do!')
        command = '#'
    elif command == 'search':
    elif command.split()[0] == 'Take':
    elif command == 'Inventory':
        print('Invalid command')

Here is the .py file for Rooms:

from GameItems import Sword, Flower, Rusty_Key, Ornate_Key, Moonstone, Stick, Knife

world['introd'] = Room('introd', "You are in a forest, you can hear wildlife all around you. There seems to be a clearing in the distance.", {'n': "clearing"}, {"Search the ground", "Go North"}, {'Sword': Sword}, None, False)

world['clearing'] = Room('clearing', "You are in a clearing surrounded by forest. Sunlight is streaming in, illuminating a bright white flower in the center of the clearing. \
To the South is the way you entered the forest. A well worn path goes to the East. In the distance a harp can be heard.", {'s': "introd", 'e': "forest path"}, {"Take flower", "Go south", "Go East"}, {'Flower': Flower}, None, False)

world['forest path'] = Room('forest path', "You begin walking down a well beaten path. The sounds of the forest surround you. Ahead you can see a fork in the road branching to the South and East.\
You can smell smoke coming from the South, and can hear a stream to the East", {'s': "cottage", 'e': "stream", 'w': "clearing"}, {"Go South", "Go East", "Go West"}, {'Stick': Stick}, None, False)

world['stream'] = Room('stream', "You come upon a relaxing stream at the edge of the woods. It looks like there is something shiny in the water. To your South is a rickety looking shack, \
to your West is the forest path you came down", {'s': "shack", 'w': "forest path"}, {"Go South", "Go West"}, {'Rusty_Key': Rusty_Key}, None, False)

world['shack'] = Room('shack', "In front of you is a shack, possibly used as an outpost for hunting. It looks dilapidated.", {'s': "inside shack", 'n': "stream"}, {"Go South", "Go North"}, None, None, False)

world['inside shack'] = Room('inside shack', "The inside of the shack is dirty. Bits of ragged fur are scattered about the floor and on a table against the back wall.\
A sharp looking knife is on the table. There is an ornate key hanging on the wall by a string.", {'n': "shack"}, {"Go North", "Take Knife", "Take Key"}, {'Knife': Knife, 'Ornate_Key': Ornate_Key}, Rusty_Key, True)

world['cottage'] = Room('cottage', "A quaint cottage sits in the middle of a small clearing, smoke drifting lazily from the chimney.", {'n': "forest path"}, {"Go north"}, {'Moonstone': Moonstone}, Ornate_Key, True)

And for GameItems:

Sword = Weapon("Sword", "A sharp looking sword. Good for fighting goblins!", 7, 5, 5)
Knife = Weapon("Knife", "A wicked looking knife, seems sharp!", 5, 7, 3)
Stick = Weapon("Stick", "You could probably hit someone with this stick if you needed to", 2, 3, 3)
Rusty_Key = DoorKeys("Rusty_Key", "A key! I wonder what it opens.", .01)
Ornate_Key = DoorKeys("Ornate_Key", "An ornate key with an engraving of a small cottage on one side", .01)
Moonstone = Items("Moonstone", "A smooth white stone that seems to radiate soft white light", .05)
Flower = Items("Flower", "A beautiful wildflower", .001)

Any advice/tips at all would be greatly appreciated. I know almost nothing about code architecture, so I would be very surprised if I'm doing everything right here.

There are no bugs that I'm currently experiencing, and everything is functioning how I want it to. I want to make sure that this is a solid foundation before I begin adding more to the game though.

  • \$\begingroup\$ Hi there, welcome to Code Review. I'm in the middle of writing a review, but it seems like your third code chunk is missing the imports. I just want to confirm that the third code snippet in your question is GameItems.py and that it contains the imports for Weapon, DoorKeys, and Items. \$\endgroup\$
    – omgimanerd
    Commented Aug 17, 2017 at 19:29
  • \$\begingroup\$ Hi, yes the third code-chunk is GameItems.py. The variables contained are what gets imported for Weapon, Doorkeys, and Items. Also thanks for the response! \$\endgroup\$ Commented Aug 17, 2017 at 19:33

2 Answers 2


welcome to Code Review and the Python language. You're off to a good start. Here are some tips to help you on your way.

  1. Consider following a style guide like PEP8. It will make your naming conventions much more consistent as well. Most developers will name classes with upper camel case, variables with lower snake case, and class methods with lower snake case.

  2. Let's talk organization and architecture. Your OOP doesn't look too bad, but here's what I would suggest:

    # GameItems.py
    class Items:
        def __init__(...):
    class DoorKeys(Items):
    class Weapon(Items):
    Sword = Weapon(...)
    Knife = Weapon(...)
    Flower = Items(...)

    This allows you to keep all your item related declarations together. Also, DoorKeys inherits from Items, but it has the same constructor and fields, which makes it seem redundant. You don't seem to be doing any typechecks with it, so you can just instantiate keys with the Items class. (Consider naming the classes Item and DoorKey, because each class represents a single instance of that object). You can then just check if the item name matches the room's required key name.

    # World.py
    from GameItems import *
    class Room:
    class Player:
    class Location:
    class Bag:
    WORLD = {}
    WORLD['introd'] = ...
    WORLD['cottage'] = ...

    This allows you to keep all the world related logic in one file. You would then create a separate Python file that takes this and runs the game.

    # Game.py
    from World import *
    def main():
        player = Player("Jeff", 100)
        command = ''
        while command != '':
            # game code here
    if __name__ == '__main__':

    One important thing here is that you should put the game logic inside an if __name__ == '__main__': guard. This prevents the logic inside from being exposed globally. This method of organization is just one way to do it. Of course, there are other ways to organize your game that make sense, but in general you should group together similar things and prevent circular dependencies.

  3. Avoid accessing member variables directly.

    if command in Location.room.exits:

    For a small code project like this, it's not that big of a problem, but in the future consider defining accessors and mutators like getExits() to prevent accidental modification.

Happy coding!


You have a circular dependency. Your main file imports GameItems and Rooms, but classes that are defined in the main file are used in both GameItems and Rooms. Items, DoorKeys, and Weapons should be moved to GameItems; Room should be moved to Rooms. world is also best defined in Rooms. As a critique of what the code does, I would print the list of available actions.


Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.