Skip to content
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.

Sign up for GitHub

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

Sea Turtles - Olive - swap meet #107

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

Sea Turtles - Olive - swap meet #107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
b6ffc31
wave 1 tests and vendor class completed
olive-lavine Apr 4, 2022
13e01b6
wave 2 tests passed
olive-lavine Apr 4, 2022
4279284
stringify wave 3
olive-lavine Apr 4, 2022
8ccc599
swap_items passes wave 3 tests
olive-lavine Apr 5, 2022
6c48871
swap first item passes wave 4 tests
olive-lavine Apr 5, 2022
c7bad3d
wave 4 tests passed
olive-lavine Apr 5, 2022
ad59f2a
updated condition descriptions
olive-lavine Apr 5, 2022
e0c9b71
get_best_by_category function passes wave 6 tests
olive-lavine Apr 5, 2022
04ee325
refactor wave 3
olive-lavine Apr 5, 2022
34c5b7a
asserts in wave 6 tests
olive-lavine Apr 5, 2022
a232045
passed wave 6 tests
olive-lavine Apr 5, 2022
24a99b3
condition descriptions
olive-lavine Apr 5, 2022
c6ba0c0
condition descriptions
olive-lavine Apr 5, 2022
4177ce9
clean up
olive-lavine Apr 5, 2022
fa88ef9
refactor inheretence for items
olive-lavine Apr 6, 2022
236188f
updated conditional logic for function returns
olive-lavine Apr 6, 2022
0d57424
conditional return order update
olive-lavine Apr 7, 2022
1a362df
refactor swapping
olive-lavine Apr 7, 2022
44fe511
final edits
olive-lavine Apr 7, 2022
78c79b5
change back swap first item for O(n) considerations
olive-lavine Apr 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
from swap_meet.vendor import Vendor
from swap_meet.item import Item
from swap_meet.clothing import Clothing


vendor = Vendor(inventory=["a", "b", "c"])
print(vendor.inventory)

hat = Item(category="clothing")
sock = Clothing(condition=3.9)
shirt = Item(category="clothing")
phone = Item(category="electronics")
fatimah = Vendor(inventory=[hat, shirt, phone])
print(fatimah.inventory)
print(hat)
print(sock.condition_description())

12 changes: 10 additions & 2 deletions swap_meet/clothing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,10 @@
class Clothing:
pass
from swap_meet.item import Item

class Clothing(Item):

def __init__(self, condition=0):
super().__init__(category="Clothing", condition=condition)


def __str__(self):
return "The finest clothing you could wear."
11 changes: 9 additions & 2 deletions swap_meet/decor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,9 @@
class Decor:
pass
from swap_meet.item import Item

class Decor(Item):

def __init__(self, condition=0):
super().__init__(category="Decor", condition=condition)

def __str__(self):
return "Something to decorate your space."
11 changes: 9 additions & 2 deletions swap_meet/electronics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,9 @@
class Electronics:
pass
from swap_meet.item import Item

class Electronics(Item):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The subclasses look great!


def __init__(self, condition=0):
super().__init__(category="Electronics", condition=condition)

def __str__(self):
return "A gadget full of buttons and secrets."
12 changes: 11 additions & 1 deletion swap_meet/item.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,12 @@
class Item:
pass

def __init__(self, category="", condition=0):
self.category = category
self.condition = condition

def __str__(self):
return "Hello World!"

def condition_description(self):
descriptions = ["no comment", "eh", "okay", "good", "great"," perfect, mwah"]
return descriptions[int(self.condition)]
Comment on lines +10 to +12
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love this as a succinct solution that works for decimals as well as integers! I'd consider formatting the list a little differently to make it a bit easier to read, even through it take more lines:

Suggested change
def condition_description(self):
descriptions = ["no comment", "eh", "okay", "good", "great"," perfect, mwah"]
return descriptions[int(self.condition)]
def condition_description(self):
descriptions = [
"no comment",
"eh",
"okay",
"good",
"great",
"perfect, mwah"
]
return descriptions[int(self.condition)]

58 changes: 57 additions & 1 deletion swap_meet/vendor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,58 @@
class Vendor:
pass

def __init__(self, inventory=None):
inventory = inventory if inventory is not None else []
self.inventory = inventory
Comment on lines +4 to +5
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice handling of the None default value using a Ternary operator! I generally stay away from overwriting the data we get from our parameters in case we ever need that info back. In this case there's no harm, but if we wanted to avoid overwriting the inventory parameter we could use the ternary expression to directly assign self.inventory's value:

Suggested change
inventory = inventory if inventory is not None else []
self.inventory = inventory
self.inventory = inventory if inventory is not None else []


def add(self, item):
self.inventory.append(item)
return item

def remove(self, item):
if item not in self.inventory:
return False
self.inventory.remove(item)
return item

def get_by_category(self, category):
items_by_category = []
for item in self.inventory:
if item.category == category:
items_by_category.append(item)
Comment on lines +18 to +21
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notice this pattern of:

result_list = []
for element in source_list:
    if some_condition(element):
        result_list.append(element)

can be rewritten using a list comprehension as:

result_list = [element for element in source_list if some_condition(element)]

Which here would look like:

items_by_category = [item for item in self.inventory if item.category == category]

At first, this may seem more difficult to read, but comprehensions are a very common python style, so try getting used to working with them!

return items_by_category

def swap_items(self, other, my_item, their_item):
if my_item not in self.inventory or their_item not in other.inventory:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this check for valid data, very succinct and easy to read!

return False
my_index = self.inventory.index(my_item)
their_index = other.inventory.index(their_item)
self.inventory[my_index] , other.inventory[their_index] = other.inventory[their_index] , self.inventory[my_index]
Comment on lines +27 to +29
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a really nice way to swap items in place in as few lines as possible. Because of the syntax & variable names, line 29 becomes really long, making it harder to read by needing to scroll, or making it harder to see the indentation at a glance by wrapping to another line. We could try breaking it up some to make it easier to read:

my_index = self.inventory.index(my_item)
their_index = other.inventory.index(their_item)
my_items = self.inventory
their_items = other.inventory
my_items[my_index] , their_items[their_index] = their_items[their_index] , my_items[my_index]

but this is still well over the PEP 8 style guidelines of 79 characters per line. Another approach would be to use the add and remove functions created in Wave 2:

self.remove(my_item)
self.add(their_item)
vendor.remove(their_item)
vendor.add(my_item)

I would also strongly consider adding a brief comment that states what line 29 is doing to make it really easy to understand what the operation is changing between the lists.

return True


def swap_first_item(self, other):
if not self.inventory or not other.inventory:
return False
self.inventory[0], other.inventory[0] = other.inventory[0] , self.inventory[0]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice choice to swap in place for an O(1) runtime!

return True


def get_best_by_category(self, category):
condition = -1
best_item = None
for item in self.inventory:
if item.category == category and item.condition > condition:
best_item = item
condition = item.condition
return best_item
Comment on lines +41 to +47
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great approach to find the best item in a single pass over the data. If we don't need to be concerned about adding another iteration over the input, we could reuse some code by pairing the get_by_category function and max to get the result:

Suggested change
condition = -1
best_item = None
for item in self.inventory:
if item.category == category and item.condition > condition:
best_item = item
condition = item.condition
return best_item
items_in_category = self.get_by_category(category)
return max(items_in_category, lambda item: item.condition)



def swap_best_by_category(self, other, my_priority, their_priority):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great approach to this function!

my_best = self.get_best_by_category(their_priority)
their_best = other.get_best_by_category(my_priority)
if not my_best or not their_best:
return False
self.swap_items(other, my_best, their_best)
return True
Comment on lines +50 to +56
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would consider adding some whitespace to visually separate logical sections of the code like the invalid data check from the main logic of the function:

Suggested change
def swap_best_by_category(self, other, my_priority, their_priority):
my_best = self.get_best_by_category(their_priority)
their_best = other.get_best_by_category(my_priority)
if not my_best or not their_best:
return False
self.swap_items(other, my_best, their_best)
return True
def swap_best_by_category(self, other, my_priority, their_priority):
my_best = self.get_best_by_category(their_priority)
their_best = other.get_best_by_category(my_priority)
if not my_best or not their_best:
return False
self.swap_items(other, my_best, their_best)
return True

This feedback applies to other functions like swap_items and swap_first_item as well.



4 changes: 2 additions & 2 deletions tests/integration_tests/test_wave_01_02_03.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
def test_integration_wave_01_02_03():
# make a vendor
vendor = Vendor()
Expand Down
4 changes: 2 additions & 2 deletions tests/integration_tests/test_wave_04_05_06.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
from swap_meet.decor import Decor
from swap_meet.electronics import Electronics

@pytest.mark.skip
@pytest.mark.integration_test
#@pytest.mark.skip
#@pytest.mark.integration_test
def test_integration_wave_04_05_06():
camila = Vendor()
valentina = Vendor()
Expand Down
17 changes: 8 additions & 9 deletions tests/unit_tests/test_wave_01.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
import pytest
from swap_meet.vendor import Vendor

@pytest.mark.skip
#@pytest.mark.skip
def test_vendor_has_inventory():
vendor = Vendor()
assert len(vendor.inventory) == 0

@pytest.mark.skip
#@pytest.mark.skip
def test_vendor_takes_optional_inventory():
inventory = ["a", "b", "c"]
vendor = Vendor(inventory=inventory)
Expand All @@ -16,7 +16,7 @@ def test_vendor_takes_optional_inventory():
assert "b" in vendor.inventory
assert "c" in vendor.inventory

@pytest.mark.skip
#@pytest.mark.skip
def test_adding_to_inventory():
vendor = Vendor()
item = "new item"
Expand All @@ -27,7 +27,7 @@ def test_adding_to_inventory():
assert item in vendor.inventory
assert result == item

@pytest.mark.skip
#@pytest.mark.skip
def test_removing_from_inventory_returns_item():
item = "item to remove"
vendor = Vendor(
Expand All @@ -40,7 +40,7 @@ def test_removing_from_inventory_returns_item():
assert item not in vendor.inventory
assert result == item

@pytest.mark.skip
#@pytest.mark.skip
def test_removing_not_found_is_false():
item = "item to remove"
vendor = Vendor(
Expand All @@ -49,7 +49,6 @@ 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 **********
# *********************************************************************
#raise Exception("Complete this test according to comments below.")
assert len(vendor.inventory) == 3
assert result == False
Comment on lines +53 to +54
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great consideration to check the inventory's length hasn't changed. We could also confirm there were no side affects that altered the individual contents of the inventory with :

assert all(item in vendor.inventory for item in ["a", "b", "c"])

or by individually checking values:

assert "a" in vendor.inventory
assert "b" in vendor.inventory
assert "c" in vendor.inventory

13 changes: 6 additions & 7 deletions tests/unit_tests/test_wave_02.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
from swap_meet.vendor import Vendor
from swap_meet.item import Item

@pytest.mark.skip
#@pytest.mark.skip
def test_items_have_blank_default_category():
item = Item()
assert item.category == ""

@pytest.mark.skip
#@pytest.mark.skip
def test_get_items_by_category():
item_a = Item(category="clothing")
item_b = Item(category="electronics")
Expand All @@ -23,7 +23,7 @@ def test_get_items_by_category():
assert item_c in items
assert item_b not in items

@pytest.mark.skip
#@pytest.mark.skip
def test_get_no_matching_items_by_category():
item_a = Item(category="clothing")
item_b = Item(category="clothing")
Expand All @@ -34,7 +34,6 @@ def test_get_no_matching_items_by_category():

items = vendor.get_by_category("electronics")

raise Exception("Complete this test according to comments below.")
# *********************************************************************
# ****** Complete Assert Portion of this test **********
# *********************************************************************
#raise Exception("Complete this test according to comments below.")
assert len(items) == 0
assert items == []
Comment on lines +38 to +39
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say both of these asserts check the same thing (whether items is empty). Since we want to know that it is a list and it is empty, we could just keep the second check assert items == [].

Aside from confirming the list is empty, what could we assert to make sure the vendor's inventory was not changed?

12 changes: 6 additions & 6 deletions tests/unit_tests/test_wave_03.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@
from swap_meet.vendor import Vendor
from swap_meet.item import Item

@pytest.mark.skip
#@pytest.mark.skip
def test_item_overrides_to_string():
item = Item()

stringified_item = str(item)

assert stringified_item == "Hello World!"

@pytest.mark.skip
#@pytest.mark.skip
def test_swap_items_returns_true():
item_a = Item(category="clothing")
item_b = Item(category="clothing")
Expand Down Expand Up @@ -38,7 +38,7 @@ def test_swap_items_returns_true():
assert item_b in jolie.inventory
assert result

@pytest.mark.skip
#@pytest.mark.skip
def test_swap_items_when_my_item_is_missing_returns_false():
item_a = Item(category="clothing")
item_b = Item(category="clothing")
Expand All @@ -65,7 +65,7 @@ def test_swap_items_when_my_item_is_missing_returns_false():
assert item_e in jolie.inventory
assert not result

@pytest.mark.skip
#@pytest.mark.skip
def test_swap_items_when_their_item_is_missing_returns_false():
item_a = Item(category="clothing")
item_b = Item(category="clothing")
Expand All @@ -92,7 +92,7 @@ def test_swap_items_when_their_item_is_missing_returns_false():
assert item_e in jolie.inventory
assert not result

@pytest.mark.skip
#@pytest.mark.skip
def test_swap_items_from_my_empty_returns_false():
fatimah = Vendor(
inventory=[]
Expand All @@ -112,7 +112,7 @@ def test_swap_items_from_my_empty_returns_false():
assert len(jolie.inventory) == 2
assert not result

@pytest.mark.skip
#@pytest.mark.skip
def test_swap_items_from_their_empty_returns_false():
item_a = Item(category="clothing")
item_b = Item(category="clothing")
Expand Down
6 changes: 3 additions & 3 deletions tests/unit_tests/test_wave_04.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from swap_meet.vendor import Vendor
from swap_meet.item import Item

@pytest.mark.skip
#@pytest.mark.skip
def test_swap_first_item_returns_true():
item_a = Item(category="clothing")
item_b = Item(category="clothing")
Expand Down Expand Up @@ -30,7 +30,7 @@ def test_swap_first_item_returns_true():
assert item_a in jolie.inventory
assert result

@pytest.mark.skip
#@pytest.mark.skip
def test_swap_first_item_from_my_empty_returns_false():
fatimah = Vendor(
inventory=[]
Expand All @@ -48,7 +48,7 @@ def test_swap_first_item_from_my_empty_returns_false():
assert len(jolie.inventory) == 2
assert not result

@pytest.mark.skip
#@pytest.mark.skip
def test_swap_first_item_from_their_empty_returns_false():
item_a = Item(category="clothing")
item_b = Item(category="clothing")
Expand Down
10 changes: 5 additions & 5 deletions tests/unit_tests/test_wave_05.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,25 @@
from swap_meet.decor import Decor
from swap_meet.electronics import Electronics

@pytest.mark.skip
#@pytest.mark.skip
def test_clothing_has_default_category_and_to_str():
cloth = Clothing()
assert cloth.category == "Clothing"
assert str(cloth) == "The finest clothing you could wear."

@pytest.mark.skip
#@pytest.mark.skip
def test_decor_has_default_category_and_to_str():
decor = Decor()
assert decor.category == "Decor"
assert str(decor) == "Something to decorate your space."

@pytest.mark.skip
#@pytest.mark.skip
def test_electronics_has_default_category_and_to_str():
electronics = Electronics()
assert electronics.category == "Electronics"
assert str(electronics) == "A gadget full of buttons and secrets."

@pytest.mark.skip
#@pytest.mark.skip
def test_items_have_condition_as_float():
items = [
Clothing(condition=3.5),
Expand All @@ -31,7 +31,7 @@ def test_items_have_condition_as_float():
for item in items:
assert item.condition == pytest.approx(3.5)

@pytest.mark.skip
#@pytest.mark.skip
def test_items_have_condition_descriptions_that_are_the_same_regardless_of_type():
items = [
Clothing(condition=5),
Expand Down
Loading