Masi Nakhjiri-Cheetahs Group #81
Conversation
There was a problem hiding this comment.
Hi Masi! Your project has been scored as green. A few high level notes:
-
Make sure to make frequent commits that have commit messages that describe what code you wrote/changed.
-
Try to make sure your spacing is consistent. I left this as a comment in your code as well but I would highly recommend checking out PEP8 - Python's style guide to get more information on best practices for spacing and whitespace.
-
There are several methods in your Vendor class where a
try-exceptblock 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 atry-exceptblock instead. :)
Nice job overall! You can find my comments in your code. Let me know if you have any questions! 😊
| # ********************************************************************* | ||
| # ****** Complete Assert Portion of this test ********** | ||
| # ********************************************************************* | ||
| assert items ==[] | ||
| assert len(items) == 0 |
There was a problem hiding this comment.
Since it's impossible for items == [] to be true and len(items) == 0 to be false, you only need one of these assert statements. They're verifying the same thing, just in two different ways.
| # - That all the correct items are in tai and jesse's inventories, including the items which were swapped from one vendor to the other | ||
| # - That all the correct items are in tai and jesse's inventories, | ||
| # including the items which were swapped from one vendor to the other | ||
| assert result |
| def __init__(self,category="",condition =0,age=0): |
There was a problem hiding this comment.
Small style note - Try to make sure you are being consistent with your spacing.
This line of code should have this spacing (spaces after commas, no spaces around equal signs):
def __init__(self, category="", condition=0, age=0)
You can read more about whitespace best practices in PEP 8 - Python's style guide
|
|
||
| def condition_description(self): | ||
| for key, value in CONDITION_DESCRIPTION.items(): |
There was a problem hiding this comment.
The power of dictionaries is that they allow us to access values in O(1) time using a key. So, rather than looping over the CONDITION_DESCRIPTION dictionary and checking to see if each item has self.condition as the key, we can plug self.condition directly into the dictionary as the key, and we'll get back the value associated with it. For example, we could do something like this:
def condition_description(self):
return CONDITION_DESCRIPTION[self.condition]
It's going to be really important to feel comfortable with using dictionaries and accessing items using a key. So, I would highly recommend reviewing dictionaries and maybe doing some extra practice with them if this still feels challenging.
| @@ -1,2 +1,25 @@ | |||
| CONDITION_DESCRIPTION = { | |||
There was a problem hiding this comment.
Small note - I think it's cool that you created a constant for the condition description dictionary, but I would actually recommend keeping this data structure inside your class definition. Because it's outside the class definition, any file that imports this class module can use the constant anyway they would like. For data that should be tied to a class, it's best to have it live inside the class definition so that it can only be accessed the way it was intended to be used.
| pass | ||
| from swap_meet.item import Item | ||
|
|
||
| class Electronics(Item): |
There was a problem hiding this comment.
Great job with wave 5 and using inheritance! Nice work hardcoding the category too 🥳
|
|
||
|
|
||
| def __str__(self) -> str: |
There was a problem hiding this comment.
Ooo! You're using type hinting here! Very cool! I would recommend being consistent with it. If you use it in one place, you should probably use it in the rest of your code too.
|
|
||
| def remove(self,item): | ||
| if item not in self.inventory: |
There was a problem hiding this comment.
This approach works, but consider how a try-except block could improve your time complexity. 🤔 💭 😌
| return None | ||
|
|
||
| return max( items, key=lambda item: item.condition ) |
There was a problem hiding this comment.
Oooo! Great usage of the max() function with a lambda function!! 🤩
| # return highest_item | ||
|
|
||
| # #### refactoring the above lines from 49-57 with Lambda |
No description provided.