C18 Lions ADSF #102
C18 Lions ADSF #102
Conversation
There was a problem hiding this comment.
Great job! Your code overall was good. There are some comments below on places where the code can be improved. For the commits, make sure to commit often and have more descriptive commit messages. Instead of saying what wave was completed, explain what functionality was added or changed. For example, "Created Item class" or "Added swap_items function".
| from swap_meet.item import Item | ||
|
|
||
| class Clothing(Item): |
| from swap_meet.item import Item | ||
|
|
||
| class Decor(Item): |
| pass | ||
| from swap_meet.item import Item | ||
|
|
||
| class Electronics(Item): |
| def __init__(self, category = "", condition=None, age = None): |
There was a problem hiding this comment.
Condition and age do not need to have None as the default, it can just be 0. We use None for things that are mutable, like lists and dictionaries. Integers are not mutable!
| from operator import ne | ||
| from swap_meet.item import Item |
There was a problem hiding this comment.
It looks like there were some imports that were accidentally added. It's good practice after you're done coding to make sure you only have the imports you need.
There was a problem hiding this comment.
Thank you, I 'll check them.
Actually, VSC added imports very often while I was doing my project.
| @@ -33,8 +34,8 @@ 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 | |||
| assert item_c in jesse.inventory | ||
| assert item_f in tai.inventory |
There was a problem hiding this comment.
Great start! It would be a good idea to check that all of the other items that we expect in the inventory are still in there as well.
There was a problem hiding this comment.
Thank you, I'm still working on the project to finish it.
| assert result == True | ||
| assert len(jesse.inventory) == 3 | ||
| assert len(tai.inventory) == 3 | ||
| assert item_f in tai.inventory | ||
| assert item_c in jesse.inventory |
| assert tai.inventory == [item_a, item_b, item_c] | ||
| assert jesse.inventory == [item_d, item_e, item_f] |
There was a problem hiding this comment.
It would be better to do the same kind of asserts you did in previous tests. == will fail if the lists are out of order. We don't know for sure how the swap works and where the new item will end up in the list. It is better to check if each of the items are in the list.
| assert result == False | ||
| assert len(tai.inventory) == 3 | ||
| assert len(jesse.inventory) == 3 | ||
| assert jesse.inventory == [item_f, item_e, item_d] | ||
| assert tai.inventory == [item_c, item_b, item_a] |
No description provided.