Ada S19 - Anh Huynh #77
Conversation
…th integration tests and test waves 5 and 6. Passed all tests
…Verified that all tests still passed
| @pytest.mark.skip | ||
| @pytest.mark.integration_test | ||
| #@pytest.mark.skip | ||
| #@pytest.mark.integration_test |
There was a problem hiding this comment.
No need to remove the integration_test mark
| @@ -143,7 +157,19 @@ def test_swap_best_by_category_reordered(): | |||
| their_priority="Decor" | |||
| ) | |||
|
|
|||
| raise Exception("Complete this test according to comments below.") | |||
| # Assert | |||
| assert result | |||
There was a problem hiding this comment.
You could do assert result is True to make sure that an actual True comes out of the function instead of just a truthy value
| assert item_c not in tai.inventory | ||
| assert item_b in tai.inventory | ||
| assert item_a in tai.inventory | ||
| assert item_f in tai.inventory | ||
| assert item_f not in jesse.inventory | ||
| assert item_e in jesse.inventory | ||
| assert item_d in jesse.inventory | ||
| assert item_c in jesse.inventory |
There was a problem hiding this comment.
I appreciate that these are written out in detail!
| class Clothing(Item): | ||
| # Define attributes for Clothing instances | ||
| def __init__(self, id = None, fabric = None, condition = None): |
There was a problem hiding this comment.
You could use the string literal default for fabric, since strings don't have the same problem that [] has.
| def __init__(self, id = None, fabric = None, condition = None): | |
| def __init__(self, id = None, fabric = "Unknown", condition = None): |
| class Decor(Item): | ||
| # Define attributes for Decor instances | ||
| def __init__(self, id = None, width = None, length = None, condition = None): |
There was a problem hiding this comment.
Integer defaults are fine, too!
| def __init__(self, id = None, width = None, length = None, condition = None): | |
| def __init__(self, id = None, width = 0, length = 0, condition = None): |
| # If attribute id in an instance has same value as attribute id in Item instance | ||
| # Return this instance. Otherwise, return None | ||
| if item.id == test_item.id: |
There was a problem hiding this comment.
I'd recommend not creating the fake Item with id. You could just make this:
| if item.id == test_item.id: | |
| if item.id == id: |
| # Remove friend's Item instance and add it to user's inventory | ||
| # Return True | ||
| else: |
There was a problem hiding this comment.
The else: isn't strictly necessary here because the return False above will exclude anything outside the if statement.
| # other_vendor.inventory.remove(friend_first_item) | ||
| # other_vendor.inventory.append(user_first_item) | ||
| # self.inventory.append(friend_first_item) |
There was a problem hiding this comment.
You can remove old code. If you commit before changing it, too, git will remember it for you.
| # Loop through each Item instance in inventory | ||
| for item in self.inventory: | ||
| # If an instance's category (aka class name) same as provided category, | ||
| # add this instance to resulting_list | ||
| if item.get_category() == category: | ||
| resulting_list.append(item) | ||
| # Return updated resulting_list | ||
| return resulting_list |
There was a problem hiding this comment.
Notice this pattern of:
result_list = []
for element in source_list:
if some_condition(element):
result_list.append(element)can be rewritten using a list comprehension as:
result_list = [element for element in source_list if some_condition(element)]Which here would look like:
resulting_list = [item for item in self.inventory if item.get_category() == category]
return resulting_listAt first, this may seem more difficult to read, but comprehensions are a very common python style, so I encourage y’all to try working with them!
| return False | ||
| else: | ||
| # Otherwise, call on swap_items method to swap user's and friend's | ||
| # matching best instances with each other | ||
| self.swap_items(other_vendor, user_best_match, other_best_match) | ||
| # return True | ||
| return True |
There was a problem hiding this comment.
Since swap_items can handle items that are None, you could also defer to it:
return self.swap_items(other_vendor, user_best_match, other_best_match)
No description provided.