Skip to content

Cheetahs - Wanjun Lan#79

Open
wjlan wants to merge 6 commits intoAda-C18:masterfrom
wjlan:master
Open

Cheetahs - Wanjun Lan#79
wjlan wants to merge 6 commits intoAda-C18:masterfrom
wjlan:master

Conversation

@wjlan
Copy link
Copy Markdown

@wjlan wjlan commented Oct 7, 2022

No description provided.

Copy link
Copy Markdown

@jbieniosek jbieniosek left a comment

Choose a reason for hiding this comment

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

Fantastic work on this project Wanjun! The code is very clean and easy to read. Good work using helper methods in the vendor class. Nice work on the age extension! This project is green.

Comment thread swap_meet/clothing.py
Comment on lines +4 to +5
def __init__(self, condition = 0, age = 0):
super().__init__(category = "Clothing", condition = condition, age = age)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

Comment thread swap_meet/item.py
elif self.condition <= 4.0:
return "heavily used"
elif self.condition <= 5.0:
return "You probably want a glove for this one..."
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 work!

Comment thread swap_meet/vendor.py
Comment on lines +15 to +17
if item in self.inventory:
self.inventory.remove(item)
return 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.

💯

Comment thread swap_meet/vendor.py
Comment on lines +22 to +24
for item in self.inventory:
if item.category == category:
result.append(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.

Great solution!

Comment thread swap_meet/vendor.py
Comment on lines +31 to +34
self.remove(my_item)
other.add(my_item)
other.remove(their_item)
self.add(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.

Good use of helper methods!

Comment thread swap_meet/vendor.py
Comment on lines +63 to +68
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)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fantastic use of helper methods!

Comment thread swap_meet/vendor.py
Comment on lines +75 to +77
my_newest_aged_item = min(self.inventory, key=lambda item: item.age)
their_newest_aged_item = min(other.inventory, key=lambda item: item.age)
self.swap_items(other, my_newest_aged_item, their_newest_aged_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.

🎉 Fantastic, great use of lambda!

Comment on lines +56 to +58
assert len(vendor.inventory) == 3
assert item not in vendor.inventory
assert result == False No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

assert item_f in tai.inventory
assert item_d in jesse.inventory
assert item_e in jesse.inventory
assert item_c in jesse.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.

💯

#### Optional tests ####

pytest.mark.skip
def test_swap_by_newest_return_true():
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 addition!

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