Scissors - Ivana #67
Scissors - Ivana #67
Conversation
- Add missing Docstrings in Item Child Classes - Add new methods to get and swap items by age - Add new test wave 7 to test new methods
| """ | ||
|
|
||
| def __init__(self, condition=0, age=0): | ||
| super() |
There was a problem hiding this comment.
This isn't actually doing anything as its written here.
There was a problem hiding this comment.
I think I get super() now! finallyyyy
| self.category = "Clothing" | ||
| self.condition = condition | ||
| self.age = age |
There was a problem hiding this comment.
This totally works but there is a tiny opportunity to DRY up your code a little but calling super().__init__() with these parameters from the child classes instead. It would be worth trying to rewrite these to utilize that if you can
| descriptions = { | ||
| 5: "Like New", | ||
| 4: "Mint", | ||
| 3: "Very Good", | ||
| 2: "Good", | ||
| 1: "Fair", | ||
| 0: "Poor" | ||
| } | ||
|
|
||
| if self.condition not in descriptions: | ||
| return None | ||
| return descriptions[self.condition] |
| if not self.inventory or not friend.inventory: | ||
| return False | ||
| else: | ||
| my_item = self.inventory.pop(0) | ||
| self.add(friend.inventory.pop(0)) | ||
| friend.add(my_item) | ||
| return True |
There was a problem hiding this comment.
This looks a lot like the code in the method above. It may be worth seeing if you can revisit this to DRY up your code by calling the other method from this one
There was a problem hiding this comment.
So it took me a while to understand how I could be able to use the swap here, in the beginning it seemed that it wouldn't do exactly what I wanted until I figured out (with the help of my tutor) that I should just index the first item in each vendors' lists on method call :)) will change this!
| best_cond = 0 | ||
| best_item = None | ||
|
|
||
| for item in self.get_by_category(item_category): |
| mine = self.get_best_by_category(their_priority) | ||
| theirs = other.get_best_by_category(my_priority) | ||
| if not mine or not theirs: | ||
| return False | ||
| else: | ||
| return self.swap_items(other, mine, theirs) |
| else: | ||
| return self.swap_items(other, mine, theirs) | ||
|
|
||
| def get_by_newest(self): |
There was a problem hiding this comment.
Love that you were able to do this optional and that you created this helper method rather than trying to do all the work in swap_by_newest!
There was a problem hiding this comment.
glad I managed to DRY this part properly!
| @@ -0,0 +1,77 @@ | |||
| import pytest | |||
There was a problem hiding this comment.
thank you SO much for this feedback, Jared! I'm sorry for taking so long to answer, I'll implement the changes as you suggested. Thanks again and have a great weekend!
There was a problem hiding this comment.
Ivana, I love that you took the time to respond to this feedback! You are definitely welcome to update your code with the suggestions I offered but I want to make sure you know you aren't expected or required to.
The feedback is first and foremost feedback for you to incorporate in future code submissions. :)
No description provided.