Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I've been trying to figure out the best way to create a inventory system in Python. I like the way this one works but what I was wondering was if there was a better way to write it.

inv = ['Sword','Armor']
item_values = {'Sword':['Sword',5,1,15,2],
               'Armor':['Armor',0,10,25,5]}
print('Name\tAtk\tArm\tLb\tVal')
print(item_values[inv[0]][0],'\t',item_values[inv[0]][1],'\t',item_values[inv[0]][2],'\t',item_values[inv[0]][3],'\t',item_values[inv[0]][4])
print(item_values[inv[1]][0],'\t',item_values[inv[1]][1],'\t',item_values[inv[1]][2],'\t',item_values[inv[1]][3],'\t',item_values[inv[1]][4])
share|improve this question
2  
What exactly do you like about the way that one works? –  jonrsharpe Jul 19 at 9:28
2  
Not directly related to the question, but tab formatting is generally a bad idea as there's no guarantee that the 'columns' will line up if certain values are longer than a 'column'. String formatting is the preferable way of formatting text. Some examples from the horse's mouth: docs.python.org/2/library/string.html#format-examples –  Pharap Jul 19 at 10:16

3 Answers 3

The items and the inventory can be modelled intuitively with classes:

  • You can define an Item class that has attributes like name, weight, attack
  • You can define an Inventory class that has a collection of items, and knows how to print them

Something like this:

class Item(object):
    def __init__(self, name, attack, armor, weight, price):
        self.name = name
        self.attack = attack
        self.armor = armor
        self.weight = weight
        self.price = price


class Inventory(object):    
    def __init__(self):
        self.items = {}

    def add_item(self, item):
        self.items[item.name] = item

    def print_items(self):
        print('\t'.join(['Name', 'Atk', 'Arm', 'Lb', 'Val']))
        for item in self.items.values():
            print('\t'.join([str(x) for x in [item.name, item.attack, item.armor, item.weight, item.price]]))


inventory = Inventory()
inventory.add_item(Item('Sword', 5, 1, 15, 2))
inventory.add_item(Item('Armor', 0, 10, 25, 5))
inventory.print_items()

UPDATE

Based on a suggestion by @jonrsharpe you could move the printing logic from print_items to the __str__ method:

def __str__(self):
    out = '\t'.join(['Name', 'Atk', 'Arm', 'Lb', 'Val'])
    for item in self.items.values():
        out += '\n' + '\t'.join([str(x) for x in [item.name, item.attack, item.armor, item.weight, item.price]])
    return out

This way, instead of inventory.print_items(), you can print the inventory more intuitively with print(inventory).

share|improve this answer
    
Note that making items a class, rather than instance, attribute could cause problems later on (for example if the Inventory is also used for a shop or if a two-player mode is added); see this SO question. You could also implement __str__ and __repr__ rather than providing an explicit printing method. –  jonrsharpe Jul 19 at 11:16
    
@jonrsharpe ouch, that was bad of me, thanks, fixed. I also considered using __str__ and __repr__, but a tricky thing with that is doing it in a way to keep the column ordering logic of the headers and the entries in one place, that's why I opted for an explicit printing method instead. –  janos Jul 19 at 11:59
    
It still seems a bit awkward to define the method for displaying an Item in the Inventory, but I see what you mean - perhaps define a class attribute Item.HEADERS? Then Inventory can print the headers followed by str(item). –  jonrsharpe Jul 19 at 15:41
    
Yeah I think it's awkward too. I think it's not a good idea after all. I don't think the __str__ methods are for fine-tuning printing. I think the __str__ method is good to write in a way that helps you in debugging. To pretty-print in some specific format, you should do it elsewhere. –  janos Jul 19 at 15:55

If you don't want to go for a full class (which may be unnecessary if you won't have methods on your inventory or item instances), your other options include dictionaries:

inventory = {'Sword': {'attack': 5, 'defence': 1,
                       'weight': 15, 'price': 2},
             'Armor': {'attack': 0, 'defence': 10,
                       'weight': 25, 'price': 5}}

for name, item in inventory.items():
    print "{0}: {1[attack]} {1[defence]} {1[weight]} {1[price]}".format(name, item)

or a collections.namedtuple:

from collections import namedtuple

Item = namedtuple('Item', ('attack', 'defence', 'weight', 'price'))
inventory = {'Sword': Item(5, 1, 15, 2),
             'Armor': Item(0, 10, 25, 5)}

for name, item in inventory.items():
    print "{0}: {1.attack} {1.defence} {1.weight} {1.price}".format(name, item)

Note the slightly different syntax for using these objects with str.format to create a more readable representation.

You can include the 'name' in either of these structures, although at present it's duplicated information. It's also not clear why you have a separate list of the names; you can just iterate through the inventory keys.

share|improve this answer

Combining ideas from the two earlier posts, you can use both a namedtuple and a custom __str__ function that formats the string for each item:

from collections import namedtuple
Item = namedtuple('Item', ('name', 'attack', 'armor', 'weight', 'price'))

item_template = '{0.name:<10}' + ''.join('{{0.{field}:<5}}'.format(field=field) 
    for field in Item._fields[1:])
Item.__str__ = lambda item: item_template.format(item)
inventory_heading = Item('Name', 'Atk', 'Arm', 'Lb', 'Val')

inventory = [
    Item('sword', 5, 1, 15, 2),
    Item('armour', 0, 10, 25, 5)
]

for item in [inventory_heading] + inventory:
    print item

Going further, you could make inventory a class that inherits from list but which has its own __str__ function too.

class Inventory(list):
    heading = Item('Name', 'Atk', 'Arm', 'Lb', 'Val')
    def __str__(self):
        return '\n'.join(map(str, [self.heading] + self))

inventory = Inventory([
    Item('sword', 5, 1, 15, 2),
    Item('armour', 0, 10, 25, 5)
])

print inventory

However, as you game gets more complicated you would probably want to move away from this type of approach to establish a division between what is displayed and the underlying game logic and data. This would tend to mean having a separate class or set of functions for managing the display output that are kept separate from your main game objects.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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