-
Notifications
You must be signed in to change notification settings - Fork 113
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
Ada c17 whales-Nishat Salsabil #96
base: master
Are you sure you want to change the base?
Conversation
…ve. All unit tests in Wave 01 passing; Wave 01 complete.
…and Waves 01-02 complete
… 03 test cases complete
… in wave 05 passing; wave 05 complete
… passing; waves 01-06 complete
…e class Item's super function in subclasses; all waves and integration tests passing
… added 2 instance methods: get_newest_item and swap_by_newest to Vendor Class, added an attribute age to Item Class
…in Wave 05, waves 01-07 and integration tests passing
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.
Fantastic work on this project, Nish! The code you wrote passed all the tests and met the learning goals. Phenomenal job on the optional enhancements and completing the tests. Your project is green 🟢 ! Congratulations on completing Unit 1!
def __init__(self, category = None, condition = 0, age = 0): | ||
if not category: |
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.
If we don't specify a category, it will be None
because of category = None
in the constructor!
|
||
|
||
def age_statement(self): |
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.
Nice addition of an age statement!
first_items = self.swap_items(other_vendor, my_item, their_item) | ||
return first_items | ||
except IndexError: |
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.
Wonderful try-except and catching IndexError
!
self.inventory.remove(item) | ||
return item | ||
return False |
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.
Nice use of short-circuiting and returning False
at the end!
my_item = other.get_best_by_category(my_priority) | ||
their_item = self.get_best_by_category(their_priority) | ||
return self.swap_items(other, their_item, my_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 use of helper functions to DRY your code!
|
||
assert newest_item | ||
assert newest_item.category == "Clothing" | ||
assert newest_item.age == pytest.approx(1) |
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.
Nice use of pytest.approx()
!
|
||
newest_item = nishat.get_newest_item() | ||
|
||
assert newest_item is 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.
👍🏻
assert item_a in nishat.inventory | ||
assert item_b not in nishat.inventory | ||
assert item_b in fatima.inventory | ||
assert item_c in nishat.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.
👍🏻
|
||
# Test 40 | ||
#@pytest.mark.skip | ||
def test_swap_newest_item_reordered(): |
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.
Awesome test idea for another ordering!
their_item = nishat.get_newest_item() | ||
) | ||
|
||
assert not result |
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.
👍🏻
An additional wave 07 was created for the optional enhancement section of the project.