Tigers - Maria and Misha #88
Conversation
There was a problem hiding this comment.
Nice job working together, Maria and Misha!
Here are a few things to consider for future projects that I've detailed below in your code:
- Consider refactoring your child classes, so that they use inheritance. It is an important skill to practice as we move forward.
If you have any questions about the feedback, please reach out to me!
swap_meet/clothing.py
Outdated
|
|
||
| class Clothing(Item): | ||
| def __init__(self, category = "Clothing", condition=0): |
There was a problem hiding this comment.
Hmm, why aren't we using inheritance for these child classes? The parent class Item already has the ability to construct the attributes category and condition. No reason to reinvent the wheel and do that work over again:
def __init__(self, condition=0):
super().__init__(category="Clothing", condition=condition)Let's break this down. super represents our parent class. We call it with () and call the method __init__. Then we pass in the necessary arguments the parent class needs to construct the attributes.
Since we know that Clothing will always have the category "Clothing", let's not give users the opportunity to change that. Remove the category parameter from the child constructor. Now, we as the coder, can always ensure Clothing class will always have a category of "Clothing".
Lastly, based on PEP8 styling guide, "Don’t use spaces around the = sign when used to indicate a keyword argument, or when used to indicate a default value for an unannotated function parameter."
| def __init__(self, category = "Clothing", condition=0): | |
| def __init__(self, category="Clothing", condition=0): |
swap_meet/decor.py
Outdated
| self.category = category | ||
| self.condition = condition |
There was a problem hiding this comment.
same as above! Let's use inheritance and remove the spaces around the default parameters
swap_meet/electronics.py
Outdated
| def __init__(self, category="Electronics", condition=0): | ||
| self.category = category | ||
| self.condition = condition |
There was a problem hiding this comment.
same as above! Let's use inheritance
| def __init__(self, category = "", condition=0): |
There was a problem hiding this comment.
| def __init__(self, category = "", condition=0): | |
| def __init__(self, category="", condition=0): |
| def condition_description(self): | ||
| rating = self.condition | ||
| description = ["very gross", "Not that great", "It's okay...", "gently used", "pretty good", "really great!"] |
There was a problem hiding this comment.
Good idea using a list to find the appropriate description based on the index position!
swap_meet/vendor.py
Outdated
| self.remove(my_item) | ||
| vendor.add(my_item) | ||
| vendor.remove(their_item) | ||
| self.add(their_item) | ||
| return True |
There was a problem hiding this comment.
same as above! Let's remove the else and let the main focus of the method stand on its own
swap_meet/vendor.py
Outdated
| return False | ||
| else: | ||
| self.swap_items(vendor, self.inventory[0], vendor.inventory[0]) |
There was a problem hiding this comment.
Nice job reusing a method!
Here is another option if we want to consider time-space efficiency. swap_meet winds up being O(n) (mostly due to removing the items from the inventory). We could simply swap the items in place here to have O(1) runtime.
self.inventory[0], vendor.inventory[0] = vendor.inventory[0], self.inventory[0]More information on swapping values with tuples
|
|
||
|
|
||
| def get_best_by_category(self, category): |
swap_meet/vendor.py
Outdated
| self.swap_items(other, my_item_to_swap, others_item_to_swap) | ||
| return True |
There was a problem hiding this comment.
same as above! Let's remove the else and let the main focus of the method stand on its own
|
|
||
|
|
||
| def add(self, added_item): |
No description provided.