C22 - Wei Qiang (Sphinx) and Jen Nguyen (Phoenix)#8
Conversation
…working but rough draft of condition_description()
…variable, added additional implementation/tests of optional enhancement
…ewest, added additional tests
yangashley
left a comment
yangashley
left a comment
There was a problem hiding this comment.
Nice job on swap-meet, Wei and Jen!
I can see from the commit history that you made frequent commits, which is great!
Let me know if you have any questions about my comments.
| @@ -1,2 +1,10 @@ | |||
| class Clothing: | |||
| pass No newline at end of file | |||
| from swap_meet.item import Item | |||
There was a problem hiding this comment.
Nice work importing Item since we're using by name as the parent class of Clothing 👍
| super().__init__(id, condition, age) | ||
| self.fabric = fabric |
There was a problem hiding this comment.
Nice work on here!
On line 5 you correctly set the default value for fabric to "Unknown".
On line 6 you properly call the initializer of the parent, and pass along the values the child wants to use.
Lastly, good work remembering to refactor the function definition to include age so that it can get set from the subclasses.
| self.fabric = fabric | ||
|
|
||
| def __str__(self): | ||
| return f"{super().__str__()} It is made from {self.fabric} fabric." |
| def __str__(self): | ||
| return f"An object of type {self.get_category()} with id {self.id}." | ||
|
|
||
| def get_category(self): |
There was a problem hiding this comment.
Nice job writing this method in the super class and not repeating the logic in the sub classes 👍
| if self.condition >= 4: | ||
| return "Excellent!" | ||
| if 2 <= self.condition < 4: | ||
| return "Good!" | ||
| return "Not the best!" |
There was a problem hiding this comment.
This implementation works, but it could be considered brittle (prone to breaking) since the data relationships are encoded here (mapping a number to a string).
What data structures do you know of that could capture such an associative relationship like this? How about a dictionary with its key/value pairs?
ITEM_CONDITIONS = {
0: "Not the best!",
...
4: "Excellent", ...
}
return ITEMS_CONDITIONS.get(self.condition)Having a clear representation of the data can make code easier to read and maintain than complex conditionals.
| # ****** Complete Assert Portion of this test ********** | ||
| # ********************************************************************* | ||
| assert items == [] | ||
| assert len(items) == 0 |
There was a problem hiding this comment.
The most specific assertion is on line 38 because it guarantees that the thing in items is actually a list that is empty. Therefore, we don't need the assertion on line 39 because we already know our list is empty
| # - That the results is truthy | ||
| # - That tai and jesse's inventories are the correct length | ||
| # - That all the correct items are in tai and jesse's inventories, including the items which were swapped from one vendor to the other | ||
| assert result |
There was a problem hiding this comment.
Instead of asserting that result is truthy, I'd use a more explicit boolean check so we know that the value referenced by result is True
| assert item_c not in tai.inventory | ||
| assert item_f not in jesse.inventory |
There was a problem hiding this comment.
We can leave these two assertions off
| # - That result is truthy | ||
| # - That tai and jesse's inventories are the correct length | ||
| # - That all the correct items are in tai and jesse's inventories, and that the items that were swapped are not there | ||
| assert result |
There was a problem hiding this comment.
Same comment as above about checking the actual value that is referenced by result. Many things could evaluate to a truthy value so being specific will test what we're actually interested in.
| # - That result is falsy | ||
| # - That tai and jesse's inventories are the correct length | ||
| # - That all the correct items are in tai and jesse's inventories | ||
| assert not result |
There was a problem hiding this comment.
Same comment as above about actually checking the value of result instead of a falsey value.
3 participants
No description provided.