customer = HumanCustomer(["4 grams of magic powder"]) print(golem.help_customer(customer))We will see this output:
('0.00', [4 grams of magic powder])
Which shows us that the order for 4 grams of magic powder was fulfilled, but the price charged to the customer was 0.00. That's not acceptable. We can't be giving things away for free. This problem is fairly straightforward to fix. in the _process_payment method, we'll want to switch from using float to Decimal, and we'll set the Decimal to round up. Also note that for the best accuracy, it's typically best to instantiate a Decimal using a string. The code below should fix this problem.
def _process_payment(self, order: List[Union[MagicPowderContainer, Book, None]]) -> str:
price = Decimal("0")
for item in order:
if isinstance(item, MagicPowderContainer):
price += Decimal(item.measure()) * \
Decimal(magic_powder_price_per_gram_sign)
elif isinstance(item, Book):
price += Decimal(item.price)
price = price.quantize(Decimal("0.01"), rounding=ROUND_UP)
return f"{price:.2f}"
Once this code change is in place, then running the example from above will output the following:
('0.01', [4 grams of magic powder])
The next problem is a little tricky, but take a look at what you get when you run this code:
print(f"Magic powder: {magic_powder_container.measure()}")
customer = HumanCustomer(["100000 grams of magic powder"])
print(golem.help_customer(customer))
print(f"Magic powder: {magic_powder_container.measure()}")
customer = OrcCustomer(["Magical Maladies"])
print(golem.help_customer(customer))
print(f"Magic powder: {magic_powder_container.measure()}")
magic_powder_container.fill(100000)
print("Filled")
print(f"Magic powder: {magic_powder_container.measure()}")
customer = HumanCustomer(["10 grams of magic powder"])
print(golem.help_customer(customer))
print(f"Magic powder: {magic_powder_container.measure()}")
customer = OrcCustomer(["10 grams of magic powder"])
print(golem.help_customer(customer))
print(f"Magic powder: {magic_powder_container.measure()}")
customer = HumanCustomer(["10 grams of magic powder"])
print(golem.help_customer(customer))
print(f"Magic powder: {magic_powder_container.measure()}")
It outputs the following:
Magic powder: 100000
('100.00', [100000 grams of magic powder])
Magic powder: 0
('0.00', [])
Magic powder: 0
Filled
Magic powder: 100000
('0.01', [10 grams of magic powder])
Magic powder: 99989
('0.01', [10 grams of magic powder])
Magic powder: 99978
('0.01', [10 grams of magic powder])
Magic powder: 99968
So what we're seeing here is the first customer comes in and places an order for all of your magic powder. The order gets fulfilled and the customer is charged 100.00.After this, the next customer, an orc, comes and tries to place an order. Because the magic powder is gone, the golem is unable to use the translation amulet, and is unable to fulfill the customer's order. The customer is given nothing, and is charged nothing. After this the magic powder container gets refilled (undoubtedly because that last orc customer came and complained to you about the service, at which point you realized that the magic powder had run out, and you refilled it from out of the back room).
After this the golem serves the next customer, a human, who asks for 10 grams of magic powder. After this interaction you check the magic powder container, expecting to find it 10 grams lighter, but instead it is 11 grams lighter. What happened? As it turns out, when the golem determined that it was unable to translate for the previous orc customer, it also failed to turn the translation amulet off. So the translation amulet is left on, and will stay on, until a translation is actually needed (such as with the next customer, which is an orc, and 11 grams are removed as expected) at which point in time the golem will finally turn it off, and things will start working as expected (such as with the customer after that, which is human, and 10 grams are removed as expected).
To fix this, after activating the translation amulet, the code that will end up using the amulet should be in a try block, and the code that deactivates the amulet should be in a finally block, like this:
def _ask_for_order(self, customer: Customer) -> List[str]:
try:
if customer.race != "human":
translation_amulet.activate()
try:
order = customer.get_order()
finally:
translation_amulet.deactivate()
else:
order = customer.get_order()
return order
except:
return []
Now it's worth noting at this point that this is not the only place that uses the translation amulet. It is also used for translating the titles of books. We could also add a try finally block there, but at this point it might be worthwhile to consider creating a function using the contextmanager annotation so that we can use the amulet via a with statement. For instance, if we were to create the following function:
@contextmanager
def use_amulet(amulet: TranslationAmulet):
amulet.activate()
try:
yield
finally:
amulet.deactivate()
Then the _ask_for_order method could be changed to this:
def _ask_for_order(self, customer: Customer) -> List[str]:
try:
if customer.race != "human":
with use_amulet(translation_amulet):
order = customer.get_order()
else:
order = customer.get_order()
return order
except:
return []
And the _find_book method could be changed to this:
def _find_book(self, title: str) -> Union[Book, None]:
for book in books:
if book.language != "english":
with use_amulet(translation_amulet):
book_title = book.get_title()
else:
book_title = book.get_title()
if title == book_title:
books.remove(book)
return book
return None
At this point if we were to rerun the example from above, we'd get the following output:
Magic powder: 100000
('100.00', [100000 grams of magic powder])
Magic powder: 0
('0.00', [])
Magic powder: 0
Filled
Magic powder: 100000
('0.01', [10 grams of magic powder])
Magic powder: 99990
('0.01', [10 grams of magic powder])
Magic powder: 99979
('0.01', [10 grams of magic powder])
Magic powder: 99969
You can now see that when the third customer is served, the magic powder is reduced by only 10 grams instead of the 11 grams that it was before.The final issue that we'll take a look at isn't a bug, but rather it's an optimization issue. When the sales golem is looking for a book to fill an order, it always starts with the first book and looks through all the books in order until it finds the book that it is looking for. Considering that the bookshelf is alphabetized, this is slow, and not only is is slow, it will use up extra magic powder translating more books than it needs to. Take for instance this example:
print(f"Magic powder: {magic_powder_container.measure()}")
customer = HumanCustomer(["Wand Maintenance"])
print(golem.help_customer(customer))
print(f"Magic powder: {magic_powder_container.measure()}")
This will print out:
Magic powder: 100000
('2.37', [Wand Maintenance])
Magic powder: 99996
Notice that it had to use 4 grams of magic powder to translate the titles of the orcish books "Adventures of Thorg" and the three copies of "Sorcery 101" before it was able to locate the book that it was looking for.On the other hand, had it been using a binary search, it would have only needed to use 1 gram of magic powder, since it would have started with the book in the middle (one of the copies of "Sorcery 101"), used a gram of magic powder to translate that title, at which point it would realize the the book needed will be to the right, and it will rule out all books to the left, then move to the middle book of the books to the right, and find the copy of "Wand Maintenance" and be done.
So how can we do this in Python? Unfortunately it's not as straightforward as I'd like it to be for this case. Python has the functions bisect and bisect_left, which will perform this kind of search, but these functions do not currently allow for a key parameter to be passed in in similar fashion to how many other Python functions work. (Note that there have been conversations to add this functionality to the bisect functions, and there is a code being written for it. See here for the conversation history, and here for the code.) If it did, then we could simply:
# This won't work because bisect_left doesn't have a key parameter
def _find_book(self, title: str) -> Union[Book, None]:
book_title = None
def key(book: Book):
nonlocal book_title
if book.language != "english":
with use_amulet(translation_amulet):
book_title = book.get_title()
else:
book_title = book.get_title()
return book_title
index = bisect_left(books, title, key=key)
if book_title == title:
book = books.pop(index)
return book
else:
return None
But since it doesn't, what are some options that we could do to make this work? The two primary recommendations are to either 1) maintain a separate list in parallel to the list that you need to search, which in this case would mean having the sales golem maintain a list of the translated titles of the books separate from the books themselves. Or 2) have the object implement the __lt__ method, which in this case would mean adding the __lt__ method to the Book class.So long as we're sticking to the strictest sense of the rules for the problem, then we would be unable to modify the Book class, which would mean going with option 1, which would potentially look something like this:
class SalesGolem:
def __init__(self):
self._book_titles = []
for book in books:
if book.language != "english":
with use_amulet(translation_amulet):
book_title = book.get_title()
else:
book_title = book.get_title()
self._book_titles.append(book_title)
...
def _find_book(self, title: str) -> Union[Book, None]:
index = bisect_left(self._book_titles, title)
book_title = self._book_titles[index]
if book_title == title:
book = books.pop(index)
self._book_titles.pop(index)
return book
else:
return None
...
If we run this with the previous example, we will get the following output:
Magic powder: 99995
('2.37', [Wand Maintenance])
Magic powder: 99995
You'll notice that this is even more expensive than the previous version, using 5 grams of magic powder because it also needed to translate the title for the orcish book "What's the Difference Between a Wizard and a Warlock?", and it had to do all this before even helping the customer. Now, for helping a single customer, this is clearly worse, but the benefits to it would add up over the course of helping multiple customers because all title translations happened up front, and does not need to be repeated for each individual customer. For instance, if we run this scenario:
print(f"Magic powder: {magic_powder_container.measure()}")
customer = HumanCustomer(["Wand Maintenance"])
print(golem.help_customer(customer))
print(f"Magic powder: {magic_powder_container.measure()}")
customer = HumanCustomer(["The Vanishing Book on Vanishing"])
print(golem.help_customer(customer))
print(f"Magic powder: {magic_powder_container.measure()}")
customer = HumanCustomer(["Sorcery 101"])
print(golem.help_customer(customer))
print(f"Magic powder: {magic_powder_container.measure()}")
customer = HumanCustomer(["Magical Maladies"])
print(golem.help_customer(customer))
print(f"Magic powder: {magic_powder_container.measure()}")
Then you'll get the following output:
Magic powder: 99995
('2.37', [Wand Maintenance])
Magic powder: 99995
('0.00', [None])
Magic powder: 99995
('15.47', [Sorcery 101])
Magic powder: 99995
('9.99', [Magical Maladies])
Magic powder: 99995
Notice that outside of the up front cost, no further cost is incurred, so this can work well, if you expect a lot of customers to be buying books. But this does still have a problem. What happens if you add another book to the shelf? Well then the sales golem's internal list of titles gets out of sync, and without resyncing the list, the golem could end up giving the customer the wrong book. Take a look at this scenario:
print(f"Magic powder: {magic_powder_container.measure()}")
customer = HumanCustomer(["Wand Maintenance"])
print(golem.help_customer(customer))
print(f"Magic powder: {magic_powder_container.measure()}")
customer = HumanCustomer(["The Vanishing Book on Vanishing"])
print(golem.help_customer(customer))
print(f"Magic powder: {magic_powder_container.measure()}")
print(f"Adding orc book, Arcane Arts")
books.insert(1, OrcishBook("Arcane Arts", "13.33"))
print(f"Magic powder: {magic_powder_container.measure()}")
customer = HumanCustomer(["Sorcery 101"])
print(golem.help_customer(customer))
print(f"Magic powder: {magic_powder_container.measure()}")
customer = HumanCustomer(["Magical Maladies"])
print(golem.help_customer(customer))
Which outputs:
Magic powder: 99995
('2.37', [Wand Maintenance])
Magic powder: 99995
('0.00', [None])
Magic powder: 99995
Adding orc book, Arcane Arts
Magic powder: 99995
('39.99', [See the Sites of Middle Earth])
Magic powder: 99995
('13.33', [Arcane Arts])
Magic powder: 99995
The last two customers both get the wrong books. This could be fixed by adding a way to resync the list, which could be something like this:
class SalesGolem:
def __init__(self):
self._book_titles = []
self.sync_book_titles()
def sync_book_titles(self):
self._book_titles = []
for book in books:
if book.language != "english":
with use_amulet(translation_amulet):
book_title = book.get_title()
else:
book_title = book.get_title()
self._book_titles.append(book_title)
...
Then the example needs to be changed to allow for the resync:
print(f"Magic powder: {magic_powder_container.measure()}")
customer = HumanCustomer(["Wand Maintenance"])
print(golem.help_customer(customer))
print(f"Magic powder: {magic_powder_container.measure()}")
customer = HumanCustomer(["The Vanishing Book on Vanishing"])
print(golem.help_customer(customer))
print(f"Magic powder: {magic_powder_container.measure()}")
print(f"Adding orc book, Arcane Arts")
books.insert(1, OrcishBook("Arcane Arts", "13.33"))
golem.sync_book_titles()
print(f"Magic powder: {magic_powder_container.measure()}")
customer = HumanCustomer(["Sorcery 101"])
print(golem.help_customer(customer))
print(f"Magic powder: {magic_powder_container.measure()}")
customer = HumanCustomer(["Magical Maladies"])
print(golem.help_customer(customer))
print(f"Magic powder: {magic_powder_container.measure()}")
This will change the output to:
Magic powder: 99995
('2.37', [Wand Maintenance])
Magic powder: 99995
('0.00', [None])
Magic powder: 99995
Adding orc book, Arcane Arts
Magic powder: 99989
('15.47', [Sorcery 101])
Magic powder: 99989
('9.99', [Magical Maladies])
Magic powder: 99989
This gets us back to getting the customers the right books, but cost of resyncing is high, and if it's something that we have to do regularly, then this solution is less than ideal.Another way to approach this is instead of putting the new book on the shelf and telling the golem to resync its internal list, we could instead add some functionality to the golem so that we can give the book to the golem, it can determine where it needs to be in it's internal list, and then it can add it to the bookshelf itself. We could this by adding the following method to the SalesGolem class:
def add_book(self, book: Book):
if book.language != "english":
with use_amulet(translation_amulet):
book_title = book.get_title()
else:
book_title = book.get_title()
index = bisect_left(self._book_titles, book_title)
self._book_titles.insert(index, book_title)
books.insert(index, book)
And then changing the scenario to use this method:
print(f"Magic powder: {magic_powder_container.measure()}")
customer = HumanCustomer(["Wand Maintenance"])
print(golem.help_customer(customer))
print(f"Magic powder: {magic_powder_container.measure()}")
customer = HumanCustomer(["The Vanishing Book on Vanishing"])
print(golem.help_customer(customer))
print(f"Magic powder: {magic_powder_container.measure()}")
print(f"Adding orc book, Arcane Arts")
golem.add_book(OrcishBook("Arcane Arts", "13.33"))
print(f"Magic powder: {magic_powder_container.measure()}")
customer = HumanCustomer(["Sorcery 101"])
print(golem.help_customer(customer))
print(f"Magic powder: {magic_powder_container.measure()}")
customer = HumanCustomer(["Magical Maladies"])
print(golem.help_customer(customer))
print(f"Magic powder: {magic_powder_container.measure()}")
Which then has this output:
Magic powder: 99995
('2.37', [Wand Maintenance])
Magic powder: 99995
('0.00', [None])
Magic powder: 99995
Adding orc book, Arcane Arts
Magic powder: 99994
('15.47', [Sorcery 101])
Magic powder: 99994
('9.99', [Magical Maladies])
Magic powder: 99994
This is much better, it only needs to use 1 more gram of magic powder when the new Orcish book is added, and if an English book is added, it won't need to use any at all.So is that it? Or could we do better? While the cached titles are nice, it would be better if we could get them lazily instead of eagerly. Is there a way that we could do this? Perhaps instead of having our internal title list being a list of strings that we have to pre populate before we can use it, we could instead fill it with proxy objects that will only get the titles as needed. So if we create this class:
class BookProxy:
def __init__(self, book: Book):
self._book = book
self._title = None
def get_title(self):
if not self._title:
if self._book.language != "english":
with use_amulet(translation_amulet):
self._title = self._book.get_title()
else:
self._title = self._book.get_title()
return self._title
def __lt__(self, other):
if isinstance(other, BookProxy):
return self.get_title() < other.get_title()
elif isinstance(other, Book):
return self.get_title() < other.get_title()
else:
return self.get_title() < other
Then we can take the SalesGolem class and modify it as follows:
class SalesGolem:
def __init__(self):
self._book_titles = [BookProxy(book) for book in books]
...
def add_book(self, book: Book):
book_proxy = BookProxy(book)
index = bisect_left(self._book_titles, book_proxy)
self._book_titles.insert(index, book_proxy)
books.insert(index, book)
def _find_book(self, title: str) -> Union[Book, None]:
index = bisect_left(self._book_titles, title)
book_proxy = self._book_titles[index]
if book_proxy.get_title() == title:
book = books.pop(index)
self._book_titles.pop(index)
return book
else:
return None
...
Then we're able to run our scenario and get back the following output:
Magic powder: 100000
('2.37', [Wand Maintenance])
Magic powder: 99999
('0.00', [None])
Magic powder: 99998
Adding orc book, Arcane Arts
Magic powder: 99996
('15.47', [Sorcery 101])
Magic powder: 99995
('9.99', [Magical Maladies])
Magic powder: 99995
At which point everything is running as expected, and the total amount of magic powder used is even less. What's more, we've also managed to optimize for the single customer scenario as well. If we run the single customer scenario from above, then we get this output:
Magic powder: 100000
('2.37', [Wand Maintenance])
Magic powder: 99999
So with that, our final SalesGolem class (and supporting functions and classes) looks like this:
class SalesGolem:
def __init__(self):
self._book_titles = [BookProxy(book) for book in books]
def help_customer(self, customer: Customer) -> (str, List[Union[MagicPowderContainer,
Book,
None]]):
order_request = self._ask_for_order(customer)
order = self._fulfill_order(order_request)
payment = self._process_payment(order)
return payment, order
def _ask_for_order(self, customer: Customer) -> List[str]:
try:
if customer.race != "human":
with use_amulet(translation_amulet):
order = customer.get_order()
else:
order = customer.get_order()
return order
except:
return []
def _fulfill_order(self, order_request: List[str]) -> List[Union[MagicPowderContainer,
Book,
None]]:
order = []
for item in order_request:
if item.endswith(" grams of magic powder"):
ordered_grams = int(item.split(" ")[0])
try:
grams = magic_powder_container.take_magic_powder(ordered_grams)
except:
grams = magic_powder_container.take_remainder()
order.append(MagicPowderContainer(grams))
else:
try:
book = self._find_book(item)
except:
book = None
order.append(book)
return order
def add_book(self, book: Book):
book_proxy = BookProxy(book)
index = bisect_left(self._book_titles, book_proxy)
self._book_titles.insert(index, book_proxy)
books.insert(index, book)
def _find_book(self, title: str) -> Union[Book, None]:
index = bisect_left(self._book_titles, title)
book_proxy = self._book_titles[index]
if book_proxy.get_title() == title:
book = books.pop(index)
self._book_titles.pop(index)
return book
else:
return None
def _process_payment(self, order: List[Union[MagicPowderContainer, Book, None]]) -> str:
price = Decimal("0")
for item in order:
if isinstance(item, MagicPowderContainer):
price += Decimal(item.measure()) * \
Decimal(magic_powder_price_per_gram_sign)
elif isinstance(item, Book):
price += Decimal(item.price)
price = price.quantize(Decimal("0.01"), rounding=ROUND_UP)
return f"{price:.2f}"
@contextmanager
def use_amulet(amulet: TranslationAmulet):
amulet.activate()
try:
yield
finally:
amulet.deactivate()
class BookProxy:
def __init__(self, book: Book):
self._book = book
self._title = None
def get_title(self):
if not self._title:
if self._book.language != "english":
with use_amulet(translation_amulet):
self._title = self._book.get_title()
else:
self._title = self._book.get_title()
return self._title
def __lt__(self, other):
if isinstance(other, BookProxy):
return self.get_title() < other.get_title()
elif isinstance(other, Book):
return self.get_title() < other.get_title()
else:
return self.get_title() < other
Now while this exercise is clearly not grounded in the real world, the issues and solutions do very much apply to real world scenarios. Using a with statement to ensure that translation amulet gets turned off can easily be applied to making sure that an open file gets closed. The issue with the finances and the use of the Decimal class to solve them can clearly be applied to finances in the real world. And minimizing the magic powder usage while speeding up the process of finding the right book could be construed to a real world example of needing to find specific resources, where some of them are local and easy to access, while others would require a network call and would be fairly expensive to make.
