Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 18 additions & 2 deletions swap_meet/clothing.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,18 @@
class Clothing:
pass
from swap_meet.item import Item

class Clothing(Item):
"""Clothing class inherits id and condition from Item class and adds the
fabric attribute.
"""

def __init__(self, fabric = "Unknown", id = None, condition = 0):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice use of default values.

super().__init__(id, condition)
self.fabric = fabric

def get_attribute(self):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I fully see what you were doing with get_attribute function, and it is a really neat solution to the issue. However, I think we're bending the guidelines of OOP design with how it's implemented. Name-wise, another developer unfamiliar with the project might have some confusion around what determines what of the several attributes the class holds is returned by a function get_attribute. If it's a function all child classes have, generally we would define it in the parent class so the children could inherit and override as necessary. I'd need to think a bit more about if there is a specific pattern that would work better, but one suggestion would be to give the parent class a function like get_comparison_key that each of the children can override.

return self.fabric

def __str__(self):
summary = super().__str__()
class_summary = f"It is made from {self.fabric} fabric."
return ". ".join((summary, class_summary))
22 changes: 20 additions & 2 deletions swap_meet/decor.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,20 @@
class Decor:
pass
from swap_meet.item import Item

class Decor(Item):
"""Decor class inherits id and condition from Item class and adds the
width and length attributes.
"""

def __init__(self, id = None, width = 0, length = 0, condition = 0):
super().__init__(id, condition)
self.width = width
self.length = length

def get_attribute(self):
item_size = self.width * self.length
return item_size

def __str__(self):
summary = super().__str__()
class_summary = f"It takes up a {self.width} by {self.length} sized space."
return ". ".join((summary, class_summary))
20 changes: 18 additions & 2 deletions swap_meet/electronics.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,18 @@
class Electronics:
pass
from swap_meet.item import Item

class Electronics(Item):
"""Electronics class inherits id and condition from Item class and adds the
type attribute.
"""

def __init__(self, id = None, type = "Unknown", condition = 0):
super().__init__(id, condition)
self.type = type

def get_attribute(self):
return self.type

def __str__(self):
summary = super().__str__()
class_summary = f"This is a {self.type} device."
return ". ".join((summary, class_summary))
31 changes: 30 additions & 1 deletion swap_meet/item.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,31 @@
import uuid

class Item:
pass
"""Item class has a unique id and a condition."""

def __init__(self, id = None, condition = 0):
if id is not None:
if type(id) is not int:
raise ValueError("invalid id input")
else:
self.id = id
else:
self.id = uuid.uuid1().int
Comment on lines +7 to +13
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think we could simplify the if/else tree a little by combining our validation checks:

if id is not None and type(id) is not int:
    raise ValueError("invalid id input")

self.id = id if id else uuid.uuid1().int

self.condition = condition

def get_category(self):
return type(self).__name__

def condition_description(self):
switch = {
0 : "gross",
1 : "not good",
2 : "okay",
3 : "good",
4 : "not bad",
5 : "excellent"
}
return switch.get(self.condition)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice handling for condition description! It looks like the code assumes that self.condition will always be an integer, but what if we allowed decimals like a condition of 3.5? How could we adapt our code to convert the decimal to an int we could use here, or alternately, change the if-statements to support decimals?


def __str__(self):
return f"An object of type {self.get_category()} with id {self.id}"
160 changes: 159 additions & 1 deletion swap_meet/vendor.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,160 @@
class Vendor:
pass
"""Vendor class contains an inventory list of Item objects and methods to
add, remove, retrieve, and swap items with other vendor instances.
"""

def __init__(self, inventory = None):
"""Creates Vendor instance"""

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

def add(self, item):
"""Adds items to inventory. Returns item."""

self.inventory.append(item)
return item

def remove(self, item):
"""Removes item from inventory. Returns removed item or None if item
not found.
"""

if item in self.inventory:
self.inventory.remove(item)
return item
return None
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I like the explicit return for None if the item is not found.


def get_by_id(self, id):
"""Gets item by item id from list of inventory. Returns Item object."""

item = next((item for item in self.inventory if item.id == id), None)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice use of next with a None default!

return item

def get_by_category(self, category):
"""Gets items in inventory that match category. Returns a list."""

inventory = self.inventory
items = [item for item in inventory if item.get_category() == category]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great use of a list comprehension!

return items

def get_best_by_category(self, category):
"""Gets item with the best condition in category from inventory. Returns
Item object.
"""

items = self.get_by_category(category)

if not items:
return None

max_value = max([item.condition for item in items])

item = next(
(item for item in items if item.condition == max_value), None)
Comment on lines +50 to +53
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I love the idea to use max! We could combine these lines if we pass max a key parameter that returns the item's condition:

return max(items, key=lambda item: item.condition)


return item

def swap_best_by_category(self, other_vendor, my_priority, their_priority):
"""Swaps best items in category from two vendors. Returns boolean."""

my_item = self.get_best_by_category(their_priority)
their_item = other_vendor.get_best_by_category(my_priority)
Comment on lines +60 to +61
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These variable names are solid, and we could be even more descriptive. Using line 60 as an example, some other options could be their_interest_item, best_item_their_interest, or my_best_of_their_priority


if not their_item or not my_item:
return False

swapped = self.swap_items(other_vendor, my_item, their_item)
return swapped

def get_by_category_attribute(self, category, attribute):
"""Gets items from inventory that match category and item attribute.
Returns a list.
"""

category_items = self.get_by_category(category)
items = [item for item in category_items
if attribute == item.get_attribute()]

return items

def swap_by_attribute(self, other_vendor, category, attribute):
"""Swaps items that match category and item attribute from 2 vendors.
Returns boolean.
"""

my_items = self.get_by_category_attribute(category, attribute)
their_items = other_vendor.get_by_category_attribute(
category, attribute)

if not their_items or not my_items:
return False

my_item = my_items[0]
their_item = their_items[0]
swapped = self.swap_items(other_vendor, my_item, their_item)
return swapped

def swap_items(self, other_vendor, my_item, their_item):
"""Swaps items from 2 vendors. Returns boolean."""

inventory = self.inventory
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think this variable could be removed, I see it referenced on the line below, but not after that.

if my_item not in inventory or their_item not in other_vendor.inventory:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

With indentation this line is a little long, I would consider breaking it up at the or keyword.

if (my_item not in self.inventory 
        or their_item not in other_vendor.inventory):
    return False

The PEP8 style guide doesn't give a strict recommendation on how to break an if-statement across multiple lines, but they give a few suggestions in the second half of the section on indentation: https://peps.python.org/pep-0008/#indentation

return False

self.inventory.remove(my_item)
other_vendor.inventory.append(my_item)

other_vendor.inventory.remove(their_item)
self.inventory.append(their_item)

return True

def swap_first_item(self, other_vendor):
"""Swaps first item in inventory from 2 vendors. Returns boolean."""

if not self.inventory or not other_vendor.inventory:
return False

my_item = self.inventory[0]
their_item = other_vendor.inventory[0]
swapped = self.swap_items(other_vendor, my_item, their_item)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Something to consider about using swap_items is that the time complexity winds up being O(n) due to removing the items from the inventory. If we were in a different scenario where it was important to lower the time complexity and the order that we add items to the inventory didn't matter, we could swap the items in place to have O(1) runtime with something like:

self.inventory[0], other_vendor.inventory[0] = other_vendor.inventory[0], self.inventory[0]

return swapped

def swap_by_id(self, other_vendor, my_item_id, their_item_id):
"""Swaps items by id from 2 vendors. Returns boolean."""

my_item = self.get_by_id(my_item_id)
their_item = other_vendor.get_by_id(their_item_id)

if not their_item or not my_item:
return False

swapped = self.swap_items(other_vendor, my_item, their_item)
return swapped

def choose_and_swap_items(self, other_vendor, category = ""):
"""Displays inventory from 2 vendors and allows user to select items to
swap by id. Returns boolean.
"""

self.display_inventory(category)
other_vendor.display_inventory(category)

my_item_id = int(input("Enter item from your inventory by id: "))
their_item_id = int(input("Enter item from their inventory by id: "))
swapped = self.swap_by_id(other_vendor, my_item_id, their_item_id)
return swapped

def display_inventory(self, category = ""):
"""Displays Vendor inventory with item descriptions."""

if not self.inventory:
print("No inventory to display.")
return

items = self.get_by_category(category) if category else self.inventory
if items:
for i, item in enumerate(items, 1):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great use for enumerate!

print(f"{i}. {str(item)}")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Because we have implemented the __str__ method, this is one of the situations where python will implicitly call it for us, we don't need to wrap item in a call to str:

print(f"{i}. {item}")

else:
print("No inventory to display.")
2 changes: 1 addition & 1 deletion tests/integration_tests/test_wave_01_02_03.py
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
@pytest.mark.integration_test
def test_integration_wave_01_02_03():
# make a vendor
Expand Down
2 changes: 1 addition & 1 deletion tests/integration_tests/test_wave_04_05_06_07.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from swap_meet.decor import Decor
from swap_meet.electronics import Electronics

@pytest.mark.skip
# @pytest.mark.skip
@pytest.mark.integration_test
def test_integration_wave_04_05_06(capfd):
camila = Vendor()
Expand Down
15 changes: 9 additions & 6 deletions tests/unit_tests/test_wave_01.py
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_returns_none():
item = "item to remove"
vendor = Vendor(
Expand All @@ -49,7 +49,10 @@ def test_removing_not_found_returns_none():

result = vendor.remove(item)

raise Exception("Complete this test according to comments below.")
# raise Exception("Complete this test according to comments below.")
# *********************************************************************
# ****** Complete Assert Portion of this test **********
# *********************************************************************
assert len(vendor.inventory) == 3
assert item not in vendor.inventory
assert result is None
Comment on lines +56 to +58
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great assertions, I would also suggest checking the inventory contents, since checks like the length being correct do not guarantee that the individual elements are still what we expect.

17 changes: 11 additions & 6 deletions tests/unit_tests/test_wave_02.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,35 @@
from swap_meet.vendor import Vendor
from swap_meet.item import Item

@pytest.mark.skip
# @pytest.mark.skip
def test_items_have_default_uuid_length_id():
item = Item()
assert isinstance(item.id, int)
assert len(str(item.id)) >= 32

@pytest.mark.skip
# @pytest.mark.skip
def test_item_instances_have_different_default_ids():
item_a = Item()
item_b = Item()
assert item_a.id != item_b.id

@pytest.mark.skip
# @pytest.mark.skip
def test_items_use_custom_id_if_passed():
item = Item(id=12345)
assert isinstance(item.id, int)
assert item.id == 12345

@pytest.mark.skip
# @pytest.mark.skip
def test_items_use_string_as_custom_id_raises_value_error():
with pytest.raises(ValueError):
item = Item(id="142")

# @pytest.mark.skip
def test_item_obj_returns_text_item_for_category():
item = Item()
assert item.get_category() == "Item"

@pytest.mark.skip
# @pytest.mark.skip
def test_get_item_by_id():
test_id = 12345
item_custom_id = Item(id=test_id)
Expand All @@ -36,7 +41,7 @@ def test_get_item_by_id():
result_item = vendor.get_by_id(test_id)
assert result_item is item_custom_id

@pytest.mark.skip
# @pytest.mark.skip
def test_get_item_by_id_no_matching():
test_id = 12345
item_a = Item()
Expand Down
Loading