-
Notifications
You must be signed in to change notification settings - Fork 590
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
Lions C18 LP Wilson Swap Meet #21
base: main
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.
Excellent job! Your code overall was great. There were a few places where the code could be simpler/cleaner. See comments below.
For commits, it would be great if the commit messages could be more descriptive. Instead of writing about which wave was completed, consider messages like "Created Item class" or "Added swap_items function".
|
||
class Clothing(Item): | ||
def __init__(self, category = "Clothing", condition = 0.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.
We actually do not want to have category in the __init__
function. We want the category to always be "Clothing", but if we put category here, the user can set category to whatever they want, which isn't good!
from swap_meet.item import Item | ||
|
||
class Decor(Item): |
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!
pass | ||
from swap_meet.item import Item | ||
|
||
class Electronics(Item): |
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!
@@ -1,2 +1,17 @@ | |||
class Item: |
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.
Excellent!
methods: .add(item), .remove(item), .get_by_category(category), | ||
swap_items(swapping_vendor, my_item, their_item)""" | ||
def __init__(self, inventory = None): |
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.
Excellent!
assert item_a not in items | ||
assert item_b not in items | ||
assert item_c not in items |
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 asserts! These are actually not needed though. Based on the first assert, we know the list is empty. We don't need to check that each of these individual items are in the list.
#assert | ||
assert result | ||
assert len(tai.inventory) == 3 | ||
assert len(jesse.inventory) == 3 | ||
|
||
assert item_f in tai.inventory | ||
assert item_a in tai.inventory | ||
assert item_b in tai.inventory | ||
|
||
@pytest.mark.skip | ||
assert item_c in jesse.inventory | ||
assert item_d in jesse.inventory | ||
assert item_e in jesse.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.
Excellent!
assert result | ||
assert len(tai.inventory) == 3 | ||
assert len(jesse.inventory) == 3 | ||
|
||
assert item_f in tai.inventory | ||
assert item_a in tai.inventory | ||
assert item_b in tai.inventory | ||
assert item_c not in tai.inventory | ||
|
||
@pytest.mark.skip | ||
assert item_c in jesse.inventory | ||
assert item_d in jesse.inventory | ||
assert item_e in jesse.inventory | ||
assert item_f not in jesse.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.
Excellent!
assert not result | ||
assert len(tai.inventory) == 3 | ||
assert len(jesse.inventory) == 3 | ||
assert item_a in tai.inventory | ||
assert item_b in tai.inventory | ||
assert item_c in tai.inventory | ||
assert item_d in jesse.inventory | ||
assert item_e in jesse.inventory | ||
assert item_f in jesse.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.
Excellent!
assert not result | ||
assert len(tai.inventory) == 3 | ||
assert len(jesse.inventory) == 3 | ||
assert item_a in tai.inventory | ||
assert item_b in tai.inventory | ||
assert item_c in tai.inventory | ||
assert item_d in jesse.inventory | ||
assert item_e in jesse.inventory | ||
assert item_f in jesse.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.
Excellent!
No description provided.