-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rock - Abigail C. #65
base: master
Are you sure you want to change the base?
Conversation
…get_by_category/Wave01 and Wave02
…Parent Class being Item / Wave05
… swap_best_by_category methods
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job Abigail! You hit all the learning goals. I added a few comments about inheritance and edge cases.
Well done!
self.category = "Clothing" | ||
self.condition = condition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! You can further refactor this by using super()
to inherit the category and condition attributes since Clothing is a child class.
self.category = "Clothing" | ||
self.condition = condition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The two lines can be reduced to
self.category = "Clothing" | |
self.condition = condition | |
super().__init__("Clothing", condition) |
self.category = "Decor" | ||
self.condition = condition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same recommendation as above to use super()
self.category = "Electronics" | ||
self.condition = condition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beating a dead horse but same as above about super()
self.category = category | ||
self.condition = condition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is condition always going to be an integer? A way to make your code more robust is to also check if condition is the correct data type you're expecting. What about float values or strings?
''' | ||
reassigns the stringified item | ||
''' | ||
return "Hello World!" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice! 👍
if self.condition == 5: | ||
five_condition_description = "Great Condition" | ||
return five_condition_description | ||
elif self.condition == 4: | ||
four_condition_description = "Good Condition" | ||
return one_condition_description | ||
elif self.condition == 3: | ||
three_condition_description = "Okay Condition" | ||
return one_condition_description | ||
elif self.condition == 2: | ||
two_condition_description = "Poor Condition" | ||
return one_condition_description | ||
elif self.condition == 1: | ||
one_condition_description = "Very Poor Condition" | ||
return one_condition_description |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One good thought exercise is how mighty our conditionals change if condition was a float?
for item in other_vendor.inventory: | ||
if item == other_vendor_item: | ||
self.inventory.append(item) | ||
other_vendor.inventory.remove(item) | ||
for item in self.inventory: | ||
if item == vendor_item: | ||
other_vendor.inventory.append(item) | ||
self.inventory.remove(item) | ||
return True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! 👍
if item.condition > largest_num: | ||
largest_num = item.condition | ||
largest_item = item | ||
return largest_item |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job! 👍
vendor_item = other.get_best_by_category(my_priority) | ||
other_item = self.get_best_by_category(their_priority) | ||
self.swap_items(other, other_item, vendor_item) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great use of other methods!
No description provided.