Sapphire - Angie Contreras C19 #70
Conversation
There was a problem hiding this comment.
Great work on this project, Angie! This project is marked as a "green" on my end.
Overall, your code demonstrated great understanding of OOP, instance methods, and inheritance. Your code was logical, clean, and efficient. In addition, your optional enhancements on the project are great-- Great work on the newest features and the tests for it.
I wish I had more specific feedback for you, but overall all I would say is to keep it up, and let me know if there are any questions!
As a small note, your git hygiene is good, and I'd prefer if the commit messages were more descriptive of the code changes within the commit (for example, "implements swap_items feature" or "handles empty inventories in swap_items" etc)
Great work overall!
| other_vendor.add(item_to_other_vendor) | ||
|
|
||
| item_from_other_vendor_to_me = other_vendor.remove(their_item) | ||
| self.add(item_from_other_vendor_to_me) |
There was a problem hiding this comment.
I really like that you used the return value of remove (item_to_other_vendor and item_from_other_vendor_to_me)! I think it's a good pattern.
There was a problem hiding this comment.
Thank you! I have to admit I was super confused throughout the project with the name of parameters and variables but I'm happy that at the end I understood the concepts and usage of these parameters inside my methods.
| my_first_item = self.inventory[0] | ||
|
|
||
| self.swap_items(other_vendor, my_first_item, first_item_from_friend) |
There was a problem hiding this comment.
Great work using the swap_items instance method.
| item_other_want = self.get_best_by_category(their_priority) | ||
|
|
||
| return self.swap_items(other_vendor, item_other_want, item_I_want) |
There was a problem hiding this comment.
Great job returning the value of swap_items-- you utilized the fact that swap_items will return True/False for you :)
| return None | ||
|
|
||
| return min(items_from_category, key=lambda item: item.age) |
| assert item_f in jesse.inventory | ||
|
|
||
|
|
||
| def test_swap_by_newest_item_no_inventory_is_false(): |
There was a problem hiding this comment.
Great work on these swap_by_newest tests!
No description provided.