C22-Phoenix-Amber Edwards & Sphinx- Weixi He#2
C22-Phoenix-Amber Edwards & Sphinx- Weixi He#2Msambere wants to merge 12 commits intoAda-C22:mainfrom
Conversation
Created functions in the vendor and item classes following the require- ments of the README: include function name, parameters, and "pass" in the body
kelsey-steven-ada
left a comment
kelsey-steven-ada
left a comment
There was a problem hiding this comment.
Great work! 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 =]
| # Testing github connection weixi | ||
| # Testing github connections |
There was a problem hiding this comment.
After testing the connection, we should remove changes like this that we don't want to bring into our final product.
| return "Item" | ||
|
|
||
| def __str__(self): | ||
| return f"An object of type Item with id {self.id}." |
There was a problem hiding this comment.
Every child class starts with a sentence of this exact form. A child class can call their parent's functions using super.
If instead of hardcoding the category Item in this string we called self.get_category(), how could we share this function with the child classes to reduce similar or repeated code in their __str__ functions?
| def condition_description(self): | ||
| descript_dict ={ | ||
| 0: "Just burn it", | ||
| 1: "Maybe re-gift to someone you don't like.", |
| 5: "Pristine!" | ||
| } | ||
|
|
||
| return descript_dict[self.condition] |
There was a problem hiding this comment.
We don't test for this, and it isn't a requirement, but something to consider:
- In the Wave 5 description the condition can be 0-5, so what happens if the condition is a decimal rather than an integer?
- How could we update the function to handle both types?
| class Vendor: | ||
| pass No newline at end of file | ||
| def __init__(self, inventory=None): | ||
| self.inventory = [] if inventory is None else inventory |
There was a problem hiding this comment.
Great use of a python ternary statement!
| @pytest.mark.skip | ||
| @pytest.mark.integration_test | ||
| #@pytest.mark.skip | ||
| #@pytest.mark.integration_test |
There was a problem hiding this comment.
This marker should not be commented out in the integration test files. For more info, check the README section for the project on Integration Tests.
| assert len(vendor.inventory) == 3 | ||
| assert result is False | ||
| assert vendor.inventory == ["a", "b", "c"] |
There was a problem hiding this comment.
Great assertions to check the return value is as-expected and that no items in the inventory were unexpectedly changed!
| item_c = Item() | ||
| fatimah = Vendor( | ||
| inventory=[item_a, item_b, item_c] | ||
| inventory=["a", "b", "c"] |
There was a problem hiding this comment.
The existing data in the "Arrange" step should not be changed when adding assertions for these tests.
- What could the assertions look like using the original data for this test?
| raise Exception("Complete this test according to comments below.") | ||
| # Assert | ||
| assert result | ||
| assert len(tai.inventory) == 3 and len(jesse.inventory) == 3 |
There was a problem hiding this comment.
When writing tests, we want to write individual assert statements for each condition we are checking. When we write multiple checks on a single line, then we need to do further investigation to figure out which expression is failing if the test fails.
| assert item_a in tai.inventory and item_b in tai.inventory and item_f in tai.inventory | ||
| assert item_d in jesse.inventory and item_e in jesse.inventory and item_c in jesse.inventory |
There was a problem hiding this comment.
These lines are a bit long. If we want to condense the checks for all items to a single line, one approach would be to convert the lists to sets and use set operations:
assert {item_f, item_a, item_b}.issubset(tai.inventory)
assert {item_c, item_d, item_e}.issubset(jesse.inventory)We could also use all:
assert all(item in tai.inventory for item in [item_f, item_a, item_b])
assert all(item in jesse.inventory for item in [item_c, item_d, item_e])2 participants
No description provided.