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.

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

Swap_meet_Daria_Iakymenko_C17(Whales) #92

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

Conversation

diakymenko
Copy link

No description provided.

Copy link

@jericahuang jericahuang left a comment

Choose a reason for hiding this comment

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

Phenomenal work on this project, Daria! You demonstrated mastery of Unit 1 concepts and did fantastic with the optional enhancements. 👍🏻 Great code style and very well done on completing the tests. Your project is green 🟢 !


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

Choose a reason for hiding this comment

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

Wonderful! Taking advantage of inheritance by using the parent class's constructor makes this concise!

Comment on lines +32 to +36
self_index = self.inventory.index(my_item)
temp_item = my_item
self.inventory[self_index] = their_item
vendor.inventory[other_index] = temp_item

Choose a reason for hiding this comment

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

Very nice implementation to literally swap by the items' index values! Another approach is using .append() and .remove() but your implementation is just as good!

def swap_first_item(self, vendor):
if self.inventory and vendor.inventory:
return self.swap_items(vendor, self.inventory[0], vendor.inventory[0])

Choose a reason for hiding this comment

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

Very nice! Takes advantage of .swap_items() return value and guard clause will prevent an IndexError 👍🏻 Love it!

Comment on lines +56 to +59
vendor_best_item = self.get_best_by_category(their_priority)
other_best_item = other.get_best_by_category(my_priority)
return self.swap_items(other, vendor_best_item, other_best_item)

Choose a reason for hiding this comment

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

Nice use of helper functions to make this concise!

Comment on lines +71 to +72
other_newest_item = min(other_category_list, key=lambda x: x.age)

Choose a reason for hiding this comment

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

Wonderful use of min() and lambda!

# *********************************************************************
# ****** Complete Assert Portion of this test **********
# *********************************************************************
assert items == [] #added assert

Choose a reason for hiding this comment

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

👍🏻

Comment on lines +61 to +69
clothing_1 = Clothing(condition=0)
clothing_2 = Clothing(condition=2)
decor = Decor(condition=3)
electronics = Electronics(condition=4)

assert clothing_1.condition_description() == "Poor"
assert clothing_2.condition_description() == "So so"
assert decor.condition_description() == "Quite good"
assert electronics.condition_description() == "Good"

Choose a reason for hiding this comment

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

👍🏻

@@ -1,10 +1,11 @@
from unicodedata import category

Choose a reason for hiding this comment

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

Not sure what this module is? Did VSCode add it mysteriously?

Comment on lines +80 to +85
assert result
assert len(jesse.inventory) == len(tai.inventory)
assert jesse.inventory[0] == item_d
assert jesse.inventory[1] == item_e
assert jesse.inventory[2] == item_c
assert tai.inventory == [item_a, item_b, item_f]

Choose a reason for hiding this comment

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

Nice!



#added 2 unit tests to check the 'swap_by_newest' function
def test_swap_by_newest_item_in_category(): #nominal case

Choose a reason for hiding this comment

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

Wonderful tests for the optional enhancement!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants