5
\$\begingroup\$

Below is my code for the Fantasy Game Inventory problem from Chapter 5 of Automate the Boring Stuff with Python. Is there anything I can do to make it cleaner or more efficient?

The code starts out with a dictionary "inv" that contains the player's current inventory as well as a list "dragon_loot" that contains the items from the dead dragon. The goal of the code is to combine the list with the dictionary while increasing the quantity of duplicate items.

As a note, I am using the starting list from the previous question simply because it seems impossible to kill a dragon without possessing a weapon :)

You are creating a fantasy video game. The data structure to model the player’s inventory will be a dictionary where the keys are string values describing the item in the inventory and the value is an integer value detailing how many of that item the player has. For example, the dictionary value {'rope': 1, 'torch': 6, 'gold coin': 42, 'dagger': 1, 'arrow': 12} means the player has 1 rope, 6 torches, 42 gold coins, and so on.

List to Dictionary Function for Fantasy Game Inventory

Imagine that a vanquished dragon’s loot is represented as a list of strings like this:

dragonLoot = ['gold coin', 'dagger', 'gold coin', 'gold coin', 'ruby']

Write a function named addToInventory(inventory, addedItems), where the inventory parameter is a dictionary representing the player’s inventory (like in the previous project) and the addedItems parameter is a list like dragonLoot. The addToInventory() function should return a dictionary that represents the updated inventory. Note that the addedItems list can contain multiples of the same item. Your code could look something like this:

def addToInventory(inventory, addedItems):
    # your code goes here

inv = {'gold coin': 42, 'rope': 1}
dragonLoot = ['gold coin', 'dagger', 'gold coin', 'gold coin', 'ruby']
inv = addToInventory(inv, dragonLoot)
displayInventory(inv)

The output of the following code is:

Inventory:
1 rope
6 torch
45 gold coin
2 dagger
12 arrow
1 ruby
    
Total number of items: 67

Thank you in advance. I have learned a lot from the comments I received for my previous questions!

from collections import Counter
    
# players current inventory before killing dragon
inv = {'rope': 1, 'torch': 6, 'gold coin': 42, 'dagger': 1, 'arrow': 12}
    
# items from the slain dragon
dragon_loot = ['gold coin', 'dagger', 'gold coin', 'gold coin', 'ruby']
    
# this function prints the players inventory in a list and reports the total items
def display_inventory(inventory):
    print("Inventory:")
    item_total = 0
    for k, v in inventory.items():
        print(str(v) + " " + str(k))
        item_total = item_total + int(v)
    print("\nTotal number of items: " + str(item_total))
    
# this function adds the dragon's stuff to the players inventory
def add_to_inventory(inventory, added_items):
        added_items_dict ={i:added_items.count(i) for i in added_items}
        inv = Counter(inventory) + Counter(added_items_dict)
        return inv
    
inv = add_to_inventory(inv, dragon_loot)
display_inventory(inv)
\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

Style

I suggest you check PEP0008 https://www.python.org/dev/peps/pep-0008/ the official Python style guide for writing a more Pythonic code and Flake8 http://flake8.pycqa.org/en/latest/ for style enforcement. The following goes accordingly:

  • Blank lines: Surround top-level function and class definitions with two blank lines. Method definitions inside a class are surrounded by a single blank line. Extra blank lines may be used (sparingly) to separate groups of related functions. Blank lines may be omitted between a bunch of related one-liners (e.g. a set of dummy implementations).

    No blank lines between the function and what's above:

    # this function prints the players inventory in a list and reports the total items
    def display_inventory(inventory):
    

    and same goes to the other function.

  • f-strings: Since you're using Python 3.x I can tell from the print statements F-strings provide a way to embed expressions inside string literals, using a minimal syntax. It should be noted that an f-string is really an expression evaluated at run time, not a constant value. You could use f-strings for readability enhancement in the following way:

    This line:

    print("\nTotal number of items: " + str(item_total))
    

    is written:

    print(f'\nTotal number of items: {item_total!s}')
    

    as well as:

    print(str(v) + " " + str(k))
    

    is written:

    print(f'{v!s} {k!s}')
    
  • Docstrings: Python documentation strings (or docstrings) provide a convenient way of associating documentation with Python modules, functions, classes, and methods. ... A docstring is simply a multi-line string, that is not assigned to anything. It is specified in source code that is used to document a specific segment of code. You should include docstrings to your functions indicating what they do and what they return.

    Example:

    def add_to_inventory(inventory: dict, added_items: list):
        """Update and return inventory."""
    

    And note the type hints to indicate the types of these parameters.

  • Indentation: your function add_to_inventory body is over indented (8 spaces instead of 4) and 4 spaces is the common convention in Python.

Code

  • A function returns: functions are meant to return, not to print.

    def display_inventory(inventory):
        print("Inventory:")
        item_total = 0
        for k, v in inventory.items():
            print(str(v) + " " + str(k))
            item_total = item_total + int(v)
        print("\nTotal number of items: " + str(item_total))
    

    And since there is no particular sophisticated printing pattern required, the display function can be replaced by print() statement(s).

  • Augmented assignment: Python supports augmented assignment using +=

    item_total = item_total + int(v) is written: item_total += int(v)

  • main guard: Use if __name__ == '__main__': at the end of your script and call your functions from there. to allow the module to be imported by other modules without running the whole script.

And the code can be a one-liner:

from collections import Counter


def update_inventory(inventory: dict, new_items: list):
    """Update and return inventory."""
    return Counter(inventory) + Counter(new_items)


if __name__ == '__main__':
    inventory = {'rope': 1, 'torch': 6, 'gold coin': 42, 'dagger': 1, 'arrow': 12}
    dragon_loot = ['gold coin', 'dagger', 'gold coin', 'gold coin', 'ruby']
    new_inventory = update_inventory(inventory, dragon_loot)
    for item, number in new_inventory.items():
        print(f'{number} {item}')
    print(f'\nTotal number of items: {sum(new_inventory.values())}')
\$\endgroup\$
3
  • 2
    \$\begingroup\$ f"{str(var)}" can also be written ass f"{var!s}", which is shorter. \$\endgroup\$
    – Gloweye
    Oct 6, 2019 at 8:28
  • \$\begingroup\$ Agreed; that needs to change. Implicit stringifying is half of the advantage of using f-strings in the first place. \$\endgroup\$
    – Reinderien
    Oct 6, 2019 at 14:20
  • 1
    \$\begingroup\$ Okay, i'll change it \$\endgroup\$
    – watch-this
    Oct 6, 2019 at 16:36
2
\$\begingroup\$

The best case would be if your inventory is always a collections.Counter. Then you could just use the update method, which takes either a dictionary-like object, or an iterable (like the constructor). This way you don't need to do added_items.count(i) all the time, which takes away almost all of the benefits of using Counter in the first place.

from collections import Counter

def print_inventory(inventory):
    """prints the players inventory
    and reports the total number of items."""
    print("Inventory:")
    item_total = 0
    for k, v in inventory.items():
        print(f"{v} {k}")
        item_total += v
    print(f"\nTotal number of items: {item_total}")


def add_to_inventory(inventory, added_items):
    """Add all items from `added_items` to `inventory`.

    `inventory` is assumed to be a `collection.Counter` instance.
    """
    inventory.update(added_items)

if __name__ == "__main__":
    # players current inventory before killing dragon
    inv = Counter({'rope': 1, 'torch': 6, 'gold coin': 42,
                   'dagger': 1, 'arrow': 12})
    # items from the slain dragon
    dragon_loot = ['gold coin', 'dagger', 'gold coin',
                   'gold coin', 'ruby']
    add_to_inventory(inv, dragon_loot)
    # inv.update(dragon_loot)  # could also just inline it...
    print_inventory(inv)

Of course, this strays a bit away from the specifications, but since this is mostly about learning how to program (or automate stuff), I think this is acceptable.

Note that I also added proper docstrings, added a if __name__ == "__main__" guard, used consistent indentation (of 4 spaces) and lower_case as required by PEP8, Python's official style-guide, used f-strings where applicable and used in-place addition for the total.

\$\endgroup\$

Your Answer

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

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