Cheetahs - Lynn Jansheski #93
Conversation
…ategory function, all other unit tests passed
…init. Cleaned up item and vendor classes.
…rect to increase code coverage to 99%
…ts to wave6 test file for new functions
…ded tests to wave6 tests file for get_newest function
…eview. Did nothing to how code works and was inadvertantly left in.
There was a problem hiding this comment.
Hi Lynn! Your project has been scored as green.
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. :)
Nice job! You can find my comments in your code. Let me know if you have any questions! 😊
| @@ -49,7 +50,9 @@ def test_removing_not_found_is_false(): | |||
|
|
|||
| result = vendor.remove(item) | |||
|
|
|||
| raise Exception("Complete this test according to comments below.") | |||
| assert result == "item to remove" | |||
There was a problem hiding this comment.
Be sure to read the instructions and test names carefully! the Vendor remove method is supposed to return False if the item is not found.
| ] | ||
|
|
||
| for i in range(len(items)): | ||
| items[i].condition_description |
There was a problem hiding this comment.
Pay attention to what each line of your code is doing and if it serves a purpose. What is this loop doing? condition_description() is a method, but this line does not call it since it's missing the ending parentheses. Even if it were calling the method, the output is not being saved anywhere to be used again.
I'd recommend doing a pass over your code after it's working to do some refactoring and clean up. :)
| @@ -52,3 +52,30 @@ def test_items_have_condition_descriptions_that_are_the_same_regardless_of_type( | |||
| assert item.condition_description() == one_condition_description | |||
|
|
|||
| assert one_condition_description != five_condition_description | |||
|
|
|||
|
|
|||
| ##########Adding tests to improve code coverage | |||
There was a problem hiding this comment.
Nice! Love that you're paying attention to code coverage! ✨
| # their inventory after swap | ||
| assert item_c in jesse.inventory | ||
| assert item_d in jesse.inventory | ||
| assert item_e in jesse.inventory |
| # ********************************************************************* | ||
| # ****** Complete Assert Portion of this test ********** | ||
| # ********************************************************************* | ||
| # Assertions should check: | ||
| # - That result is falsy | ||
| # - That tai and jesse's inventories are the correct length | ||
| # - That all the correct items are in tai and jesse's inventories | ||
|
|
||
| #############Adding tests for newest items |
There was a problem hiding this comment.
Nice job writing your own tests for newest items! 🥳
| 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 remove(self, item): | ||
| '''Remove item from inventory list.''' | ||
| if item 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 | ||
| else: | ||
| return max(self.get_by_category(category), key=lambda item: item.condition) |
There was a problem hiding this comment.
Oooo! Great usage of the max() function with a lambda function!! 🤩
| def get_newest_item(self): | ||
| '''Get newest item from inventory.''' | ||
| if len(self.inventory) > 0: |
There was a problem hiding this comment.
Since a list with 1 or more values is truthy, the Pythonic way to write this if statement would be:
if self.inventory:
😊 🐍
| return False | ||
|
|
||
| def swap_by_newest(self, other): |
There was a problem hiding this comment.
Great job with the optional enhancements! 🎉
No description provided.