Sharks - Ivana M. #100
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| from swap_meet.vendor import Vendor | ||
| from swap_meet.item import Item | ||
|
|
||
|
|
||
| vendor = Vendor() | ||
| # vendor_1 = Vendor(inventory=["item_1","item_2","item_3"]) | ||
|
|
||
| item = Item() | ||
| # item_1 = Item(category='decor') | ||
| # item_2 = Item(category='clothing', condition=3) | ||
|
|
||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,11 @@ | ||
| from swap_meet.item import Item | ||
|
|
||
| class Clothing(Item): | ||
| def __init__(self, condition=0): | ||
| super().__init__(category="Clothing", condition=condition) | ||
|
|
||
|
|
||
| def __str__(self): | ||
| return "The finest clothing you could wear." | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,9 @@ | ||
| from swap_meet.item import Item | ||
|
|
||
| class Decor(Item): | ||
| def __init__(self, condition=0): | ||
| super().__init__(category="Decor", condition=condition) | ||
|
|
||
|
|
||
| def __str__(self): | ||
| return "Something to decorate your space." |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,13 @@ | ||
| from pytest import Item | ||
|
There was a problem hiding this comment. I think this is a stray import that can be removed since you don't use pytest in Electronics |
||
|
|
||
|
|
||
| from swap_meet.item import Item | ||
|
|
||
| class Electronics(Item): | ||
| def __init__(self, condition=0): | ||
| super().__init__(category="Electronics", condition=condition) | ||
|
|
||
|
|
||
| def __str__(self): | ||
| return "A gadget full of buttons and secrets." | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,19 @@ | ||
| class Item: | ||
| def __init__(self, category="", condition=0): | ||
| self.category = category | ||
| self.condition = condition | ||
|
|
||
|
|
||
| def __str__(self): | ||
| return "Hello World!" | ||
|
|
||
|
|
||
| def condition_description(self): | ||
|
|
||
| descriptions = ["Destroyed", | ||
| "Barely there", | ||
| "Heavily used", | ||
| "Good", | ||
| "Mint", | ||
| "New"] | ||
|
|
||
| return descriptions[self.condition] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,77 @@ | ||
| class Vendor: | ||
| def __init__(self, inventory=None): | ||
| if not inventory: | ||
| inventory = [] | ||
| self.inventory = inventory | ||
|
Comment on lines
+3
to
+5
There was a problem hiding this comment. 👍 This can be re-written as a ternary, as:
In this situation, that would look like self.inventory = [] if inventory is None else inventory |
||
|
|
||
|
|
||
| def add(self, item): | ||
| self.inventory.append(item) | ||
| return item | ||
|
|
||
|
|
||
| def remove(self, item): | ||
| if item in self.inventory: | ||
| self.inventory.remove(item) | ||
| return item | ||
| else: | ||
| return False | ||
|
Comment on lines
+14
to
+18
There was a problem hiding this comment. A more explicit way to if item not in self.inventory:
return False
self.inventory.remove(item)
return itemAnother approach we could take is to try to remove the item directly, and handle the ValueError that occurs if it's not there, and return False to handle it (try/except) |
||
|
|
||
|
|
||
| def get_by_category(self, category): | ||
| items_by_category = [] | ||
|
|
||
| for item in self.inventory: | ||
| if item.category == category: | ||
| items_by_category.append(item) | ||
| return items_by_category | ||
|
Comment on lines
+22
to
+27
There was a problem hiding this comment. Looks good. This method is a great candidate for using list comprehension if you want to refactor it since our conditional statement we check is pretty simple. General syntax for list comp: result_list = [element for element in source_list if some_condition(element)]Which here would look like: items_by_category = [item for item in self.inventory if item.category == category]You can also forgo saving the list to the variable items_by_category and do something like: def get_by_category(self, category):
return [item for item in self.inventory if item.category == category] |
||
|
|
||
|
|
||
| def swap_items(self, friend, my_item, their_item): | ||
| if my_item not in self.inventory or their_item not in friend.inventory: | ||
| return False | ||
|
Comment on lines
+31
to
+32
|
||
|
|
||
| self.remove(my_item) | ||
| friend.add(my_item) | ||
|
|
||
| friend.remove(their_item) | ||
| self.add(their_item) | ||
|
|
||
| return True | ||
|
|
||
|
|
||
| def swap_first_item(self, friend): | ||
| if not self.inventory or not friend.inventory: | ||
| return False | ||
|
|
||
| self.swap_items(friend, self.inventory[0], friend.inventory[0]) | ||
| return True | ||
|
|
||
|
|
||
| def get_best_by_category(self, category): | ||
| if not self.inventory: | ||
| return None | ||
|
Comment on lines
+52
to
+53
|
||
|
|
||
| category_items = self.get_by_category(category) | ||
|
There was a problem hiding this comment. Good work calling a method you already wrote |
||
|
|
||
| if not category_items: | ||
| return None | ||
|
|
||
| best_item = category_items[0] | ||
|
Comment on lines
+57
to
+60
There was a problem hiding this comment. You can remove this check here on line 57 & 58. If there is nothing in category_items, then when you loop nothing will happen. However, you set So if category_items is empty, you would incorrectly return an item. However, you can do |
||
|
|
||
| for item in category_items: | ||
| if item.condition > best_item.condition: | ||
| best_item = item | ||
|
|
||
| return best_item | ||
|
|
||
|
|
||
| def swap_best_by_category(self, other, my_priority, their_priority): | ||
| my_best_item = self.get_best_by_category(their_priority) | ||
| their_best_item = other.get_best_by_category(my_priority) | ||
|
|
||
| if their_best_item == None or my_best_item == None: | ||
|
There was a problem hiding this comment. Because of how swap_items is implemented, checking if my_best_item and their_best_item are valid here isn't necessary. See line 31 in vendor.py - you have a check in there that makes sure the args passed into swap_items() are valid. Also swap_items() returns True if swapping happened and False if no swapping happened. Therefore, you can leverage the return value from swap_items() and refactor this method so it looks like this: def swap_best_by_category(self, other, my_priority, their_priority):
my_best_item = self.get_best_by_category(their_priority)
their_best_item = other.get_best_by_category(my_priority)
return self.swap_items(other, my_best_item, their_best_item) |
||
| return False | ||
| else: | ||
| self.swap_items(other, my_best_item, their_best_item) | ||
| return True | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Since you have the category and condition parameters in the correct order when you pass it to the parent's constructor, you can also write line 5 like:
super().__init__("Clothing", condition)