Whales_GabyWebb #95
Whales_GabyWebb #95
Conversation
| def __init__(self, category="Clothing", condition=0): | ||
| super().__init__(category, condition) |
There was a problem hiding this comment.
Great use of super here! Consider removing the category from the constructor arguments on line 3. We know we always want it to be "Clothing" so we don't want to allow changing it when instantiating an instance.
| '''returns string description of item condition''' | ||
| if self.condition == 0: | ||
| description = "Trash" | ||
| elif self.condition == 1: | ||
| description = "It Works" | ||
| elif self.condition == 2: | ||
| description = "Decent" | ||
| elif self.condition == 3: | ||
| description = "Good" | ||
| elif self.condition == 4: | ||
| description = "Like New" | ||
| elif self.condition == 5: | ||
| description = "New" | ||
| return description |
There was a problem hiding this comment.
Great job having this be defined here once and inherited by the child classes!
| self.inventory = inventory | ||
| if self.inventory is None: | ||
| self.inventory = [] |
There was a problem hiding this comment.
Nice job avoiding a mutable default argument!
| '''adds item to inventory''' | ||
| self.inventory.append(item) | ||
| return item |
| '''swaps items between vendor inventories''' | ||
| if my_item not in self.inventory or their_item not in vendor.inventory: | ||
| return False | ||
| else: | ||
| self.inventory.remove(my_item) | ||
| vendor.inventory.append(my_item) | ||
| vendor.inventory.remove(their_item) | ||
| self.inventory.append(their_item) | ||
| return True |
There was a problem hiding this comment.
Great logic! One small thing: the else is unneeded here because the if block returns and ends the function if it's entered.
| ''' | ||
| swaps items between vendor inventories | ||
| by first position in inventory | ||
| ''' | ||
| if self.inventory and vendor.inventory: | ||
| return self.swap_items(vendor, self.inventory[0], \ | ||
| vendor.inventory[0]) |
There was a problem hiding this comment.
Love how you re-use your earlier method! One small bug here: according to the README this method should return False if either of the inventories is empty, but it actually returns None.
| ''' | ||
| swaps best condition items of a given category | ||
| between vendor inventories | ||
| ''' | ||
| if not self.inventory or not other.inventory: | ||
| return False | ||
| elif not self.get_by_category(their_priority) or \ | ||
| not other.get_by_category(my_priority): | ||
| return False | ||
| else: | ||
| self_best_item = self.get_best_by_category(their_priority) | ||
| their_best_item = other.get_best_by_category(my_priority) | ||
| self.swap_items(other, self_best_item, their_best_item) | ||
| return True |
There was a problem hiding this comment.
Great logic! Similar note to earlier how the else is not needed.
| assert len(items) == 0 | ||
| assert item_a not in items | ||
| assert item_b not in items | ||
| assert item_c not in items |
There was a problem hiding this comment.
I like the idea of these asserts, but I think the last 3 are unneeded. If the length of items is 0, we already know none of the specified items are inside it.
| assert result == True | ||
| # - That tai and jesse's inventories are the correct length | ||
| assert len(tai.inventory) == 3 | ||
| assert len(jesse.inventory) == 3 | ||
| # - That all the correct items are in tai and jesse's inventories, including the items which were swapped from one vendor to the other | ||
| assert item_f in tai.inventory | ||
| assert item_a in tai.inventory | ||
| assert item_b in tai.inventory | ||
| assert item_c in jesse.inventory | ||
| assert item_d in jesse.inventory | ||
| assert item_e in jesse.inventory |
No description provided.