-
Notifications
You must be signed in to change notification settings - Fork 110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cheetahs 18 - Alyssa R. #80
base: master
Are you sure you want to change the base?
Conversation
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.
Hi Alyssa! Your project has been scored as green.
Great job making frequent commits with descriptive commit messages! ✨
There are several methods in your Vendor class where a try-except
block could be implemented to improve time complexity. Checking to see if an item is in a list before doing something with it, is an O(n) operation. So, see if it's possible to replace those if statements with a try-except
block instead. :)
Really nice job with your project! I can tell that you took the time to refactor and clean up your code. It looks great! You can find my comments in your code. Let me know if you have any questions. 😊
@@ -34,7 +34,11 @@ def test_get_no_matching_items_by_category(): | |||
|
|||
items = vendor.get_by_category("electronics") | |||
|
|||
raise Exception("Complete this test according to comments below.") | |||
assert len(items) == 0 |
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.
Good asserts 👍🏾
assert result | ||
assert len(tai.inventory) == 3 | ||
assert len(jesse.inventory) == 3 | ||
assert all(item in [item_a, item_b, item_f] for item in tai.inventory) |
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.
Niiiice! Beautiful usage of the all()
function and a generator expression!
Small recommendation - if you used a set instead of a list for item_a, item_b, item_f
, you would improve your time complexity for that item in
operation.
} | ||
if self.condition in condition_dict: | ||
return condition_dict[self.condition] |
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.
Great job with your Item
class! Love that you implemented a dictionary for the condition descriptions and also have some validation to make sure self.condition
is a key in the dictionary. 🥳
def __init__(self, inventory=None): | ||
'''A blueprint for different store inventories.''' |
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.
Beautiful docstrings! ✨
class Electronics(Item): | ||
|
||
def __init__(self, condition=0): | ||
super().__init__("Electronics", condition) |
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.
Great job with wave 5 and using inheritance! Nice work hardcoding the category too 🥳
def get_by_category(self, category): | ||
'''Return a list of items in the inventory with category.''' | ||
category_list = [i for i in self.inventory if i.category == category] |
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.
Great list comprehension! 👏🏾
Be sure to use descriptive variable names here too though!
def remove(self, item): | ||
'''Remove an item from inventory list.''' | ||
if item not in self.inventory: |
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.
This approach works, but consider how a try-except
block could improve your time complexity. 🤔 💭 😌
def get_best_by_category(self, category): | ||
'''Returns item with the best condition in a certain category.''' | ||
items_in_category = self.get_by_category(category) |
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.
Nice usage of an already implemented function 👍🏾
if not items_in_category: | ||
return None | ||
best_item = max(items_in_category, key=lambda item: item.condition) |
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.
Oooo! Great usage of the max()
function with a lambda function!! 🤩
No description provided.