Conversation
kelsey-steven-ada
left a comment
kelsey-steven-ada
left a comment
There was a problem hiding this comment.
Great work Eva! I've left a mix of suggestions and questions to consider for feedback. Please reply here on Github or reach out on Slack if there's anything I can clarify =]
|
|
||
|
|
||
| def __str__(self): | ||
| return f"{super().__str__()}. This is a {self.type} device." No newline at end of file |
There was a problem hiding this comment.
Really nice use of inheritance across the child classes.
| if id is None: | ||
| self.id = uuid.uuid4().int | ||
| else: | ||
| self.id = id |
There was a problem hiding this comment.
Nice solution, another option here could use conditional assignment:
# Assigns self.id to the parameter id if id is truthy,
# otherwise assigns self.id the return value of uuid.uuid4().int
self.id = id if id else uuid.uuid4().int| 4 : "sample item", | ||
| 5 : "brand new" | ||
| } | ||
| return level_explained[int(self.condition)] |
There was a problem hiding this comment.
I like the use of int() to account for decimal condition values.
| if item not in self.inventory: | ||
| return None |
There was a problem hiding this comment.
Nice check to ensure the Vendor doesn't contain duplicates
|
|
||
|
|
||
| def get_by_id(self, id): | ||
| print(f"\nid: {id}\n") |
There was a problem hiding this comment.
We want to remove debugging code like print statements before opening PRs.
| my_id_item = self.get_by_id(my_item_id) | ||
| other_id_item = other_vendor.get_by_id(their_item_id) | ||
|
|
||
| if my_id_item == None or other_id_item == None: |
There was a problem hiding this comment.
I want to share a reminder that in python we should use is or is not when comparing against None.
| self_id = 0 | ||
| other_id = 0 | ||
|
|
||
| while flag: |
There was a problem hiding this comment.
Nice loop to prompt for valid input!
| @@ -0,0 +1,90 @@ | |||
| import pytest | |||
| assert len(vendor.inventory) == 3 | ||
| assert vendor.inventory == ["a", "b", "c"] | ||
| assert result is None |
There was a problem hiding this comment.
Great assertions, very complete checks =]
| assert result is False | ||
| assert len(fatimah.inventory) == 3 | ||
| assert len(jolie.inventory) == 0 |
There was a problem hiding this comment.
Great assertions, I would also suggest checking fatima's inventory contents, since checks like the length being correct do not guarantee that the individual elements are still what we expect.
2 participants
No description provided.