swap_meet_Sumitra Chhetri, Scissor class #60
Conversation
There was a problem hiding this comment.
Great work on your first OOP project. You've met the learning goals of reading tests, using inheritance and composition, and using helper methods. Your code is logical and readable. I've left a few comments of ways you could consider refactoring. Keep up the hard work!
| def __init__(self, category = "Clothing", condition = 0): | ||
| self.category = "Clothing" | ||
| self.condition = condition | ||
|
|
||
| super().__init__("Clothing", condition) |
There was a problem hiding this comment.
You've done the same work multiple times, consider refactoring like this:
| def __init__(self, category = "Clothing", condition = 0): | |
| self.category = "Clothing" | |
| self.condition = condition | |
| super().__init__("Clothing", condition) | |
| def __init__(self, condition = 0): | |
| super().__init__(condition, category="Clothing") |
| if item in self.inventory: | ||
| self.inventory.remove(item) | ||
| else: | ||
| return False |
There was a problem hiding this comment.
Consider refactoring to address the edge case first:
| if item in self.inventory: | |
| self.inventory.remove(item) | |
| else: | |
| return False | |
| if item not in self.inventory: | |
| return False | |
| self.inventory.remove(item) |
| input: adds and removes items from each Vendor inventory | ||
| output: Return True or False | ||
| """ | ||
| if my_item in self.inventory and their_item in friend_vendor.inventory: |
There was a problem hiding this comment.
Consider refactoring to check the edge case first (similar to the comment for remove
| self.remove(own_item) | ||
| friend.add(own_item) | ||
| friend.remove(friend_item) | ||
| self.add(friend_item) |
There was a problem hiding this comment.
You can make use of swap_items here!
| if category_by_list == []: | ||
| return None |
There was a problem hiding this comment.
consider initializing best_item = None which removes the need for this conditional test.
| return best_item | ||
|
|
||
|
|
||
| def swap_best_by_category(self, other, my_priority, their_priority): |
There was a problem hiding this comment.
This function is clear and concise! Great work.
No description provided.