-
Notifications
You must be signed in to change notification settings - Fork 21
#Ocelots Megan Maple #22
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
base: main
Are you sure you want to change the base?
Conversation
|
|
||
| #Instantiate with id, allowing custom or using UUID Int auto generate | ||
| def __init__(self, id=None, condition=None): | ||
| self.id = id if id is not None else uuid.uuid1().int |
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 use of ternary operator
| self.condition = condition if condition is not None else 0 | ||
| #get the class name | ||
| def get_category(self): | ||
| return str(type(self).__name__) |
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 use of python introspection!
| def __str__(self): | ||
| return (f"An object of type {self.get_category()} with id {self.id}") | ||
|
|
||
| #adding a condition description class to describe condition based on value of 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.
typo. "function", not "class"
| return (f"An object of type {self.get_category()} with id {self.id}") | ||
|
|
||
| #adding a condition description class to describe condition based on value of condition | ||
| def condition_description(self): |
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.
clever, but unnecessarily so here. A dict would have sufficed, and been O(1) lookup, vs O(n) to fall through n conditions in the worse case :)
| #adding a condition description class to describe condition based on value of condition | ||
| def condition_description(self): | ||
| case = lambda x: self.condition < x | ||
| if case(1): item_condition_desc = "Very Poor Indeed" |
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.
love the descriptions, though! :D
| def remove(self, item_to_remove): | ||
| if item_to_remove not in self.inventory: | ||
| return None | ||
| self.inventory.remove(item_to_remove) |
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 inventory.remove instead of re-implementing that code.
| if item_to_give not in self.inventory or item_to_get not in vendor_friend.inventory: | ||
| return False | ||
|
|
||
| vendor_friend.add(self.remove(item_to_give)) |
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.
concise! for debugging and maintenance, this could be on different lines. That said, you did do error checking above, so that ameliorates some of the chances that you'll hit an error here.
| #get items from inventory by catagory | ||
| def get_by_category(self, category_string): | ||
|
|
||
| return [item for item in self.inventory if item.get_category() == category_string] |
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.
excellent use of list comprehension
|
|
||
| def get_best_by_category(self, category_string): | ||
| if self.get_by_category(category_string): | ||
| return max(self.get_by_category(category_string), key=lambda item: item.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.
Yay, using max plus lambda here!
No description provided.