Conversation
alope107
left a comment
alope107
left a comment
There was a problem hiding this comment.
This looks great! Nice logic and nice use of inheritance! This project is definitely green~
| class Clothing (Item): | ||
| def __init__ (self, category=str(), condition = 0): | ||
| self.category = "Clothing" | ||
| self.condition = condition |
There was a problem hiding this comment.
Here and in the other Item subclasses, the category=str() is not needed. You hardcode the category to "Clothing" on line 5, so the category that gets passed in as an argument is ignored, and thus unneeded.
Also, consider using super instead of copying the logic to set the category and condition. Even though it doesn't save you much here, it makes your code easier to extend if you ever add more complex logic to the Item constructor.
| def __str__ (self): | ||
| return "The finest clothing you could wear." |
| def condition_description(self): | ||
| if self.condition == 0: | ||
| return "This would be better served on a pedestal in a museum" | ||
| elif self.condition == 1: | ||
| return "I mean...It works, mostly" | ||
| elif self.condition == 2: | ||
| return "This is...acceptable" | ||
| elif self.condition == 3: | ||
| return "Passable for sure" | ||
| elif self.condition == 4: | ||
| return "Lightly used, a couple scuffs, but still in good shape!" | ||
| elif self.condition == 5: | ||
| return "Like new!" |
There was a problem hiding this comment.
Nice job defining this in the superclass and letting the subclasses inherit it.
| def __init__(self, inventory = None): | ||
| if inventory == None: | ||
| self.inventory = [] | ||
| else: | ||
| self.inventory = inventory |
There was a problem hiding this comment.
Good job avoiding a mutable default value! One small nitpick, use is None when checking whether an object is None.
| def remove(self, item): | ||
| if item in self.inventory: | ||
| self.inventory.remove(item) | ||
| return item | ||
| return False |
There was a problem hiding this comment.
Really nice job recognizing you don't need an else here!
| def swap_best_by_category(self, other, my_priority, their_priority): | ||
| their_offer = other.get_best_by_category(my_priority) | ||
| my_offer = self.get_best_by_category(their_priority) | ||
| if their_offer == None or my_offer == None: | ||
| return False | ||
| else: | ||
| self.swap_items(other, my_offer, their_offer) | ||
| return True No newline at end of file |
| assert result == False | ||
| assert len(vendor.inventory) == 3 | ||
| assert item not in vendor.inventory |
| assert len(items) == 0 | ||
| assert item_b not in items |
There was a problem hiding this comment.
The first assert is great here, but the second one is unneeded. If the length of the items is 0, we can assume that item_b is not in items.
| assert result is True | ||
| assert len(jesse.inventory) == 3 | ||
| assert len(tai.inventory) == 3 | ||
| assert item_f in tai.inventory | ||
| assert item_c in jesse.inventory |
| assert result is False | ||
| assert len(jesse.inventory) == 3 | ||
| assert len(tai.inventory) == 3 | ||
| assert item_d and item_e and item_f in jesse.inventory | ||
| assert item_a and item_b and item_c in tai.inventory |
There was a problem hiding this comment.
These asserts have the right idea, but there's a subtle bug in them. Take a look at this snippet and see if you can figure out how this same unintuitive behavior could cause problems in your code:
data = [1, 2, 3]
a = 1
b = 99
print(b and a in data)
# Prints True
2 participants
No description provided.