Sharks, Kassidy Buslach Swap Meet #97
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,10 @@ | ||
| from swap_meet.item import Item | ||
|
|
||
|
|
||
| class Clothing(Item): | ||
| def __init__(self,category ='', condition = 0): | ||
| super().__init__(category = "Clothing", condition = condition) | ||
| def __str__(self, category = "Clothing"): | ||
| category = "The finest clothing you could wear." | ||
| return category | ||
|
Comment on lines
+8
to
+9
There was a problem hiding this comment. Feel free to directly return the string literal like this: def __str__(self):
return "The finest clothing you could wear." |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,10 @@ | ||
| from pyparsing import condition_as_parse_action | ||
|
There was a problem hiding this comment. You can remove unused imports to keep your code clean |
||
| from swap_meet.item import Item | ||
| class Decor(Item): | ||
| def __init__(self, condition = 0 ): | ||
| super().__init__(category= "Decor", condition = condition) | ||
| def __str__(self, category= "Decor"): | ||
| self.category = "Something to decorate your space." | ||
| return self.category | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,13 @@ | ||
| from sqlite3 import connect | ||
|
|
||
| from swap_meet.item import Item | ||
| class Electronics(Item): | ||
| def __init__(self, condition = 0): | ||
| super().__init__(category= "Electronics", condition = condition) | ||
|
|
||
| def __str__(self, category= "Electronics"): | ||
| self.category = "A gadget full of buttons and secrets." | ||
| return self.category | ||
|
|
||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,26 @@ | ||
|
|
||
| from swap_meet.vendor import Vendor | ||
|
|
||
| class Item: | ||
| def __init__(self, category = "", condition = 0): | ||
| self.category = category | ||
| self.condition = condition | ||
|
|
||
| def __str__(self): | ||
| item ="Hello World!" | ||
|
There was a problem hiding this comment. Whitespace should be added after equal sign for consistency |
||
| return item | ||
|
|
||
| def condition_description(self): | ||
| if self.condition == 0: | ||
| return "About to fall to pieces." | ||
| elif self.condition == 1: | ||
| return "This is a fixer-upper." | ||
| elif self.condition == 2: | ||
| return "Needs a little bit of love." | ||
| elif self.condition == 3: | ||
| return "It works, but has been well loved." | ||
| elif self.condition == 4: | ||
| return "Good condition" | ||
| elif self.condition == 5: | ||
| return "Pristine, impeccable, immaculate!!!" | ||
|
Comment on lines
+14
to
+25
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,77 @@ | ||
|
|
||
| class Vendor: | ||
| def __init__(self, inventory= None): | ||
| inventory = inventory if inventory is not None else [] | ||
| self.inventory = inventory | ||
|
Comment on lines
+4
to
+5
There was a problem hiding this comment. Nice ternary here, you can do the assignment all in one line like this too: |
||
|
|
||
|
|
||
| def add(self, item): | ||
| self.inventory.append(item) | ||
| return item | ||
|
|
||
| def remove(self, item_to_remove): | ||
| if not self.inventory: | ||
| return False | ||
|
Comment on lines
+13
to
+14
|
||
| elif item_to_remove in self.inventory: | ||
| item_index = self.inventory.index(item_to_remove) | ||
| popped_item = self.inventory.pop(item_index) | ||
| return popped_item | ||
| else: | ||
| return False | ||
|
Comment on lines
+15
to
+20
There was a problem hiding this comment. Since you already have a guard clause then you don't need line 15, 19, 20. Getting rid of your elif statement means that the logic to remove something can be unindented one level. Also note that you can directly remove The refactor would look like this: def remove(self, item_to_remove):
if not self.inventory:
return False
self.inventory.remove(item_to_remove)
return item_to_remove |
||
|
|
||
| def get_by_category(self, category): | ||
| category_list = [] | ||
| for item in self.inventory: | ||
| if item.category == category: | ||
| category_list.append(item) | ||
| return category_list | ||
|
Comment on lines
+23
to
+27
There was a problem hiding this comment. Nice!. 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: category_list = [item for item in self.inventory if item.category == category]You can also forgo saving the list to the variable category_list 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, Vendor, my_item, their_item): | ||
|
There was a problem hiding this comment. Here you passed in the class In Python we don't need to specify types so the But that's not recommended in Python. The point of "duck typing" is that you write a function that takes "anything that acts like a Here's a resource on duck typing if you're interested in reading more: https://realpython.com/lessons/duck-typing/#:~:text=Duck%20typing%20is%20a%20concept,a%20given%20method%20or%20attribute. |
||
| if my_item not in self.inventory or their_item not in Vendor.inventory: | ||
| return False | ||
| else: | ||
| Vendor.inventory.append(self.remove(my_item)) | ||
| self.inventory.append(Vendor.remove(their_item)) | ||
| return True | ||
|
Comment on lines
+33
to
+35
There was a problem hiding this comment. Since you have a guard clause, you don't need this dangling |
||
|
|
||
| def swap_first_item(self, Vendor): | ||
| if not Vendor.inventory or not self.inventory: | ||
| return False | ||
| else: | ||
|
There was a problem hiding this comment. Since you have a guard clause, you can remove the dangling |
||
| self.swap_items(Vendor, self.inventory[0], Vendor.inventory[0]) | ||
|
There was a problem hiding this comment. Nice work reusing your swap_items method |
||
| return True | ||
|
|
||
| """Refactored Swap_first_item() post Swap Meet Wrap up | ||
| def swap_first_item(self, Vendor): | ||
| if not Vendor.inventory or not self.inventory: | ||
| return False | ||
| else: | ||
| ven_inventory = Vendor.inventory[0] | ||
| self_inventory = self.inventory[0] | ||
| Vendor.inventory.append(self.remove(self_inventory)) | ||
| self.inventory.append(Vendor.remove(ven_inventory)) | ||
| return True""" | ||
|
|
||
|
|
||
| def get_best_by_category(self, category): | ||
| categorized_list= self.get_by_category(category) | ||
| if not categorized_list: | ||
| return None | ||
| best_condition = categorized_list[0] | ||
| for item in categorized_list: | ||
| if item.condition > best_condition.condition: | ||
| best_condition = item | ||
| return best_condition | ||
|
Comment on lines
+58
to
+64
There was a problem hiding this comment. Nice! What you have here works! Suggesting another way you might see this written. A refactor can remove lines 58 and 59. To do so, you'd need to assign If If The refactor would look like: def get_best_by_category(self, category):
categorized_list = self.get_by_category(category)
best_condition = None
for item in categorized_list:
if item.condition > best_condition.condition:
best_condition = item
return best_condition |
||
|
|
||
|
|
||
| def swap_best_by_category(self, other, my_priority, their_priority): | ||
| vendors_best= other.get_best_by_category(my_priority) | ||
|
There was a problem hiding this comment. Add a whitespace to the left of the equal sign for consistency |
||
| my_best = self.get_best_by_category(their_priority) | ||
| if not vendors_best or not my_best: | ||
| return False | ||
| else: | ||
| self.swap_items(other, my_best, vendors_best) | ||
| return True | ||
|
Comment on lines
+70
to
+74
There was a problem hiding this comment. Because of how swap_items is implemented, checking if my_best and vendors_best are valid here isn't necessary. See line 30 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):
vendors_best = other.get_best_by_category(my_priority)
my_best = self.get_best_by_category(their_priority)
return self.swap_items(other, my_best, vendors_best) |
||
|
|
||
|
|
||
|
|
||
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.
White space should be consistent with the Python style guide.
Here we have some missing white spaces. this line can be refactored to look like:
Where there is white space after the comma and white space after the equal sign
You can read more here if you'd like: https://peps.python.org/pep-0008/#whitespace-in-expressions-and-statements