-
Notifications
You must be signed in to change notification settings - Fork 99
Zoisite Katherine G and Say R #79
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?
Changes from all commits
96de9be
be0f993
0cb1c85
14fc5bc
8469fd7
62e5b2c
4b4855f
2cbdc4f
b60804d
6acb7f1
fb27ffd
496be4e
8b86ec4
efb330b
7a1f546
3649570
1e8ab64
c8b64d7
b3d3ef2
d214b66
5eef2f2
accbafe
d82cb38
a304387
0939b77
d7585c2
a226137
7b8a3d6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,12 @@ | ||
| class Clothing: | ||
| pass | ||
| from .item import Item | ||
|
|
||
|
|
||
| class Clothing(Item): | ||
| def __init__(self, id = None, age = 0.0, condition = 0.0, fabric = "Unknown"): | ||
| super().__init__(id= id, age = age, condition = condition) | ||
| self.fabric = fabric | ||
|
|
||
|
|
||
| def __str__(self): | ||
| parent_str = super().__str__() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great use of the superclass method! |
||
| return (f'{parent_str} It is made from {self.fabric} fabric.') | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,12 @@ | ||
| class Decor: | ||
| pass | ||
| from .item import Item | ||
|
|
||
| class Decor(Item): | ||
| def __init__(self, id = None, age = 0.0, condition = 0.0, width = 0, length = 0): | ||
| super().__init__(id = id, age = age, condition = condition) | ||
| self.width = width | ||
| self.length = length | ||
|
|
||
|
|
||
| def __str__(self): | ||
| parent_str = super().__str__() | ||
| return (f'{parent_str} It takes up a {self.width} by {self.length} sized space.') |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,13 @@ | ||
| class Electronics: | ||
| pass | ||
| from .item import Item | ||
|
|
||
| class Electronics(Item): | ||
|
|
||
| def __init__(self, id = None, age = 0.0, condition = 0.0, type = "Unknown"): | ||
| super().__init__(id = id, age = age, condition = condition) | ||
| self.type = type | ||
|
|
||
| def __str__(self): | ||
| parent_str = super().__str__() | ||
| return (f'{parent_str} This is a {self.type} device.') | ||
|
|
||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,48 @@ | ||
| import uuid | ||
|
|
||
| class Item: | ||
| pass | ||
| def __init__(self, age = 0.0 , id = None, condition = 0.0): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tiny style nitpick: |
||
| if id is None: | ||
| self.id = uuid.uuid4().int | ||
| else: | ||
| self.id = id | ||
| self.condition = condition | ||
| self.age = age | ||
|
|
||
| def __str__(self): | ||
| return f'An object of type {self.__class__.__name__} with id {self.id}.' | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How could we use |
||
|
|
||
| def get_category(self): | ||
| return self.__class__.__name__ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great way to look up the class name dynamically! |
||
|
|
||
| def get_age(self): | ||
| return self.age | ||
|
|
||
| def condition_description(self): | ||
| if self.condition >= 0.0 and self.condition < 1.0: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great considerations in the conditionals for if self.condition is a decimal! |
||
| return "Should you get this? IT DEPENDS! ʕ •ᴥ• ʔ" | ||
| elif self.condition >= 1.0 and self.condition < 2.0: | ||
| return "One person's garbage is another's treasure." | ||
| elif self.condition >= 2.0 and self.condition < 3.0: | ||
| return "If you squint, you can't even tell." | ||
| elif self.condition >= 3.0 and self.condition < 4.0: | ||
| return "This is a bargan for the price." | ||
| elif self.condition >= 4.0 and self.condition < 5.0: | ||
| return "This item has been well cared for, but you can tell you're not the first owner." | ||
| elif self.condition >= 5.0: | ||
| return "This item is immaculate. If you don't buy it, I will!" | ||
|
Comment on lines
+22
to
+33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another approach for finding the condition description could be to make a dictionary that maps conditions as keys to descriptions as values. There's a trade off of using more memory creating the map variable, but if memory isn't a concern, for me, it's a little easier to read. condition_map = {
0: "Not Usable”,
1: "Gross",
2: "Wear and Tear",
3: "Not Great",
4: "Gently used",
5: "Like new"
}
return condition_map[int(self.condition)] |
||
|
|
||
| def age_description(self): | ||
| if self.age >= 0.0 and self.age < 1.0: | ||
| return "I've had this for less than a year!" | ||
| elif self.age >= 1.0 and self.age < 2.0: | ||
| return "I've had this for a little over a year!" | ||
| elif self.age >= 2.0 and self.age < 3.0: | ||
| return "I've had this for over two years but not more than three!" | ||
| elif self.age >= 3.0 and self.age < 4.0: | ||
| return "I've had this for over three years but not more than four!" | ||
| elif self.age >= 4.0 and self.age < 5.0: | ||
| return "I've had this for over four years but not more than five!" | ||
| elif self.age >= 5.0: | ||
| return "I've had this for over five years!" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,92 @@ | ||
| from .item import Item | ||
| from .decor import Decor | ||
| from .electronics import Electronics | ||
| from .clothing import Clothing | ||
|
|
||
| class Vendor: | ||
| pass | ||
| def __init__(self, inventory = None): | ||
| self.inventory = [] if inventory is None else inventory | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great use for a ternary operator! |
||
|
|
||
| def add(self, item): | ||
| self.inventory.append(item) | ||
| return item | ||
|
|
||
| def remove(self, item): | ||
| if item not in self.inventory: | ||
| return False | ||
| else: | ||
| self.inventory.remove(item) | ||
| return item | ||
|
Comment on lines
+15
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The if item not in self.inventory:
return False
self.inventory.remove(item)
return item |
||
|
|
||
| def get_by_id(self, id): | ||
| for item in self.inventory: | ||
| if item.id == id: | ||
| return item | ||
| if isinstance(id, Item): | ||
| raise ValueError("id should be an int!") | ||
|
Comment on lines
+25
to
+26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does the expression |
||
|
|
||
|
|
||
| def swap_items(self, other_vendor, my_item, their_item): | ||
| if not self.inventory or not other_vendor.inventory: | ||
| return False | ||
| if their_item not in self.inventory and my_item not in other_vendor.inventory: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a small bug in this line that our test data doesn't account for.
|
||
| my_index, their_index = self.inventory.index(my_item), other_vendor.inventory.index(their_item) | ||
| self.inventory[my_index], other_vendor.inventory[their_index] = their_item, my_item | ||
|
Comment on lines
+33
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To keep the line lengths under 79 characters and make these statements easy to read at a glance, I highly recommend splitting both the variable declarations and assignments into their own lines. |
||
| return True | ||
| return False | ||
|
|
||
|
|
||
| def swap_first_item(self, other_vendor): | ||
| if not self.inventory or not other_vendor.inventory: | ||
| return False | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would consider adding a blank line after validation checks like this to give readers a visual separation between sections of our code that are doing slightly different things. |
||
| self.inventory[0], other_vendor.inventory[0] = (other_vendor.inventory[0], self.inventory[0]) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Neat way to get O(1) time complexity for this function! This is another line I recommend refactoring for length, one option could be pulling the right side values into variables: my_item = self.inventory[0]
their_item = other_vendor.inventory[0]
self.inventory[0], other_vendor.inventory[0] = their_item, my_item |
||
| return True | ||
|
|
||
| def get_by_category(self, category): | ||
| matching_items = [] | ||
| matching_items = [item for item in self.inventory if item.get_category() == category] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great place for a list comprehension! |
||
| if matching_items: | ||
| return matching_items | ||
| else: | ||
| return [] | ||
|
Comment on lines
+48
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see an opportunity to reduce the code a little, do we need to create a new empty list on line 51 if |
||
|
|
||
| def get_best_by_category(self, category): | ||
| items = self.get_by_category(category) | ||
| if not items: | ||
| return None | ||
| return max(items, key=lambda item: item.condition) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Love the use of |
||
|
|
||
| def swap_best_by_category(self, other_vendor, my_priority, their_priority): | ||
| my_best_item = self.get_best_by_category(their_priority) | ||
| their_best_item = other_vendor.get_best_by_category(my_priority) | ||
| if my_best_item and their_best_item: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we change this into a guard statement to let us unindent the intended flow of the function? |
||
| if self.swap_items(other_vendor, my_best_item, their_best_item): | ||
| return True | ||
| return False | ||
|
|
||
| def get_by_age(self, age): | ||
| matching_items = [] | ||
| matching_items = [item for item in self.inventory if item.get_age() == age] | ||
| if matching_items: | ||
| return matching_items | ||
| else: | ||
| return None | ||
|
|
||
| def get_best_by_age(self, vendor): | ||
| items = vendor.inventory | ||
| if not items: | ||
| return None | ||
| return min(items, key=lambda item: item.age) | ||
|
|
||
| def swap_best_by_age(self, other_vendor): | ||
| my_newest_item = self.get_best_by_age(self) | ||
| their_newest_item = other_vendor.get_best_by_age(other_vendor) | ||
| if my_newest_item and their_newest_item: | ||
| if self.swap_items(other_vendor, my_newest_item, their_newest_item): | ||
| return True | ||
| return False | ||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,8 +2,8 @@ | |
| from swap_meet.vendor import Vendor | ||
| from swap_meet.item import Item | ||
|
|
||
| @pytest.mark.skip | ||
| @pytest.mark.integration_test | ||
| # @pytest.mark.skip | ||
| # @pytest.mark.integration_test | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For integration tests, we want to leave the decorator |
||
| def test_integration_wave_01_02_03(): | ||
| # make a vendor | ||
| vendor = Vendor() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| import pytest | ||
| from swap_meet.item import Item | ||
| from swap_meet.vendor import Vendor | ||
| from swap_meet.clothing import Clothing | ||
| from swap_meet.decor import Decor | ||
| from swap_meet.electronics import Electronics | ||
|
|
||
| def test_age_is_assigned_to_item_instances(): | ||
| # Act | ||
| item_a = Clothing(age=2.0) | ||
| item_b = Decor(age=3.0) | ||
| item_c = Electronics(age=4.0) | ||
| tai = Vendor( | ||
| inventory=[item_a, item_b] | ||
| ) | ||
|
|
||
| item_a_age, item_b_age, item_c_age = item_a.age, item_b.age, item_c.age | ||
|
|
||
| assert item_a_age == 2.0 | ||
| assert item_b_age == 3.0 | ||
| assert item_c_age == 4.0 | ||
|
|
||
| def test_get_age_returns_correct_age_value(): | ||
| item_a = Clothing(age=2.0) | ||
| item_b = Decor(age=3.0) | ||
| tai = Vendor( | ||
| inventory=[item_a, item_b] | ||
| ) | ||
| result = item_a.get_age() | ||
|
|
||
| assert result == 2.0 | ||
|
|
||
| def test_get_by_age_returns_items_with_matching_age(): | ||
| item_a = Clothing() | ||
| item_a.age = 2.0 | ||
| item_b = Electronics() | ||
| item_b.age = 3.0 | ||
| item_c = Decor() | ||
| item_c.age = 3.0 | ||
| vendor = Vendor( | ||
| inventory=[item_a, item_b, item_c] | ||
| ) | ||
|
|
||
|
|
||
| items = vendor.get_by_age(3.0) | ||
|
|
||
| assert item_b in items | ||
| assert item_c in items | ||
| assert item_a not in items | ||
|
|
||
| def test_get_best_by_age_return_newest_item(): | ||
| item_a = Clothing() | ||
| item_a.age = 2.0 | ||
| item_b = Clothing() | ||
| item_b.age = 3.0 | ||
| item_c = Clothing() | ||
| item_c.age = 4.0 | ||
| item_d = Electronics() | ||
| item_d.age = 1.0 | ||
| vendor = Vendor( | ||
| inventory=[item_a, item_b, item_c, item_d] | ||
| ) | ||
|
|
||
| result = vendor.get_best_by_age(vendor) | ||
|
|
||
| assert result == item_d | ||
|
|
||
| def test_swap_best_by_age_returns_True(): | ||
| item_a = Clothing() | ||
| item_a.age = 2.0 | ||
| item_b = Clothing() | ||
| item_b.age = 3.0 | ||
| item_c = Clothing() | ||
| item_c.age = 4.0 | ||
| vendor = Vendor( | ||
| inventory=[item_a, item_b, item_c] | ||
| ) | ||
| item_d = Clothing() | ||
| item_d.age = 4.0 | ||
| item_e = Clothing() | ||
| item_e.age = 3.0 | ||
| item_f = Clothing() | ||
| item_f.age = 2.0 | ||
| james = Vendor( | ||
| inventory=[item_d, item_e, item_f] | ||
| ) | ||
| result = vendor.swap_best_by_age(james) | ||
|
|
||
| assert result | ||
| assert item_a in james.inventory | ||
| assert item_f in vendor.inventory | ||
|
|
||
| def test_items_have_age_descriptions_that_are_the_same_regardless_of_type(): | ||
| items = [ | ||
| Clothing(age=5), | ||
| Decor(age=5), | ||
| Electronics(age=5) | ||
| ] | ||
| five_age_description = items[0].age_description() | ||
| assert isinstance(five_age_description, str) | ||
| for item in items: | ||
| assert item.age_description() == five_age_description | ||
| items[0].age = 1 | ||
| one_age_description = items[0].age_description() | ||
| assert isinstance(one_age_description, str) | ||
| for item in items: | ||
| item.age = 1 | ||
| assert item.age_description() == one_age_description | ||
|
|
||
| assert one_age_description != five_age_description |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,12 +2,12 @@ | |
| import pytest | ||
| from swap_meet.vendor import Vendor | ||
|
|
||
| @pytest.mark.skip | ||
|
|
||
| def test_vendor_has_inventory(): | ||
| vendor = Vendor() | ||
| assert len(vendor.inventory) == 0 | ||
|
|
||
| @pytest.mark.skip | ||
|
|
||
| def test_vendor_takes_optional_inventory(): | ||
| inventory = ["a", "b", "c"] | ||
| vendor = Vendor(inventory=inventory) | ||
|
|
@@ -16,7 +16,7 @@ def test_vendor_takes_optional_inventory(): | |
| assert "b" in vendor.inventory | ||
| assert "c" in vendor.inventory | ||
|
|
||
| @pytest.mark.skip | ||
|
|
||
| def test_adding_to_inventory(): | ||
| vendor = Vendor() | ||
| item = "new item" | ||
|
|
@@ -27,7 +27,7 @@ def test_adding_to_inventory(): | |
| assert item in vendor.inventory | ||
| assert result == item | ||
|
|
||
| @pytest.mark.skip | ||
|
|
||
| def test_removing_from_inventory_returns_item(): | ||
| item = "item to remove" | ||
| vendor = Vendor( | ||
|
|
@@ -40,7 +40,7 @@ def test_removing_from_inventory_returns_item(): | |
| assert item not in vendor.inventory | ||
| assert result == item | ||
|
|
||
| @pytest.mark.skip | ||
|
|
||
| def test_removing_not_found_is_false(): | ||
| item = "item to remove" | ||
| vendor = Vendor( | ||
|
|
@@ -49,7 +49,4 @@ def test_removing_not_found_is_false(): | |
|
|
||
| result = vendor.remove(item) | ||
|
|
||
| raise Exception("Complete this test according to comments below.") | ||
| # ********************************************************************* | ||
| # ****** Complete Assert Portion of this test ********** | ||
| # ********************************************************************* | ||
| assert result == False | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Something to consider, if the function returns
|
||
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 subclass constructors!