Puja Mukherjee-Lions #82
Conversation
… changed parameters of first part of Wave 1 from list to if and None
… testcase of wave 5
…lso added a testcase for swap by newest
| def __init__(self, category = "",condition = 0): | ||
| super().__init__(category = "Clothing", condition = condition) |
There was a problem hiding this comment.
Nice use of super! Here, it would be a good idea to not have category be a parameter in the Clothing constructor. We know that we always want category to be "Clothing", so we shouldn't give the caller the illusion that they can change it.
Also a small style note: when using default/named parameters, do not put spaces before or after the equals sign. It should instead look like this:
condition=0.
| return "The finest clothing you could wear." | ||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Please delete empty lines at the end of your file.
| condition_descriptions = ["I dont like zero", "I dont like one either", | ||
| "Maybe I like two", "Hmmmm.....probably three is nice", "I like four", | ||
| "Five is a nice number"] | ||
| return condition_descriptions[self.condition] |
There was a problem hiding this comment.
This works if the condition is an integer, but what would happen if it were a float? (Some of the test data shows the condition as 3.5. How could you change this method to accommodate that?
swap_meet/vendor.py
Outdated
| class Vendor: | ||
| pass No newline at end of file | ||
| MAX_AGE = 1000 | ||
| # Wave 1 | ||
| def __init__(self, inventory = None): | ||
| if inventory is None: | ||
| inventory = [] | ||
| self.inventory = inventory | ||
| def add(self,item): | ||
| self.inventory.append(item) | ||
| return item | ||
| def remove(self,item): | ||
| for inventory_item in self.inventory: | ||
| if inventory_item == item: | ||
| self.inventory.remove(item) | ||
| return item | ||
| return False |
There was a problem hiding this comment.
Great logic! Stylistically, please add an empty line between functions/methods.
| MAX_AGE = 1000 |
There was a problem hiding this comment.
What would happen if we had a dinosaur fossil that was more than a thousand years old? To accommodate this case, we can set our max age to infinity using float('inf'). We can also consider doing this in the method itself instead of having a constant attached to our Vendor. The constant is just an implementation detail that helps us find the youngest object, it's not something that we need to expose to other parts of out code. I really like the idea of using a constant though!
| my_best_item = self.get_best_by_category(their_priority) | ||
| their_best_item = other.get_best_by_category(my_priority) | ||
| if my_best_item == None or their_best_item == None: | ||
| return False | ||
| return self.swap_items(other, my_best_item, their_best_item) |
There was a problem hiding this comment.
Really nice use of helpers! Small note, please use is None instead of == None.
| vendor_newest_item = self.find_newest_item() | ||
| friend_newest_item = friend_vendor.find_newest_item() | ||
| return self.swap_items(friend_vendor, vendor_newest_item, friend_newest_item) |
| assert len(items) == 0 | ||
| assert item_a not in items | ||
| assert item_c not in items | ||
| assert item_b not in items |
There was a problem hiding this comment.
This works, but we can make it simpler by removing lines 38-40. If the length of items is 0, we know that none of our items can be present in it.
| assert (item_f in tai.inventory) == True | ||
| assert (item_c not in tai.inventory) == True | ||
| assert (item_c in jesse.inventory) == True | ||
| assert (item_f not in jesse.inventory) == True |
There was a problem hiding this comment.
We don't need the == True here. asserts automatically succeed when given a truthy value and automatically fail when given a falsey value.
| def test_swap_by_newest_returns_True(): | ||
| # Arrange | ||
| item_a = Item(age = 5) | ||
| item_b = Item(age = 1) | ||
| item_c = Item(age = 13) | ||
| fatimah = Vendor( | ||
| inventory=[item_a, item_b, item_c] | ||
| ) | ||
|
|
||
| item_d = Item(age = 6) | ||
| item_e = Item(age = 2) | ||
| jolie = Vendor( | ||
| inventory=[item_d, item_e] | ||
| ) | ||
| # Act | ||
|
|
||
| result = fatimah.swap_items(jolie, item_b, item_e) | ||
|
|
||
| # Assert | ||
|
|
||
| assert len(fatimah.inventory) == 3 | ||
| assert item_b not in fatimah.inventory | ||
| assert item_a in fatimah.inventory | ||
| assert item_c in fatimah.inventory | ||
| assert item_e in fatimah.inventory | ||
| assert len(jolie.inventory) == 2 | ||
| assert item_e not in jolie.inventory | ||
| assert item_d in jolie.inventory | ||
| assert item_b in jolie.inventory | ||
| assert result |
This project helped me learn more about classes and inheritance. Really enjoyed it!