Skip to content

Cheetahs - Ariam Yonas#100

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

Cheetahs - Ariam Yonas#100
arusphere wants to merge 6 commits intoAda-C18:masterfrom
arusphere:master

Conversation

@arusphere
Copy link

No description provided.

Copy link

@goeunpark goeunpark left a comment

Choose a reason for hiding this comment

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

Good work, Ariam! 🎉

Nice job with the first five waves; your commit messages are detailed and your code is a breeze to read! Your integration tests for the first three waves pass flawlessly and I see you were making good progress on Wave 5 as well! I would love to see you get more comfortable with writing inheritance and overriding methods since Unit 2 will rely heavily on writing classes. This project is a Yellow. 🟡

Please let me know if I can clear up any concepts or answer questions about the feedback during our 1:1s or office hours! You got this! 💯

class Clothing:
pass No newline at end of file
def __init__(self, condition = 0):
super().__init__(category="Clothing", condition = condition)

Choose a reason for hiding this comment

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

We use a super dunder init but we forgot to pass in Item as a parent class on L4!

Comment on lines +4 to +5
def __init__(self,condition = 0):
super().__init__(condition=condition, category="Decor")

Choose a reason for hiding this comment

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

Great constructor here! ✨

Comment on lines +13 to +14
if item not in self.inventory:
return False

Choose a reason for hiding this comment

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

Great validation check here! 💯

result = vendor.remove(item)

raise Exception("Complete this test according to comments below.")
assert result == False

Choose a reason for hiding this comment

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

👍🏻

super().__init__(category="Electronics", condition = condition)

def __str__(self):
return " A gadget full of buttons and secrets."

Choose a reason for hiding this comment

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

Remember that strings are sensitive about whitespace! The space between the first " and A gadget full of buttons and secrets makes a test in Wave 5 test_electronics_has_default_category_and_to_str() fail when it should pass. 😉

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

def condition_rank(self):

Choose a reason for hiding this comment

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

Make sure your method names follow the project requirements to pass the tests! Although condition_rank is a fine name, the test and the readme in Wave 5 wanted condition_description. If we change the name, the final test in Wave 5 should pass.

return category_list

def swap_items(self, friend, my_item, their_item):
if my_item in self.inventory and their_item in friend.inventory:

Choose a reason for hiding this comment

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

Brilliant chained conditionals! Love to see it!

Comment on lines +27 to +30
friend.inventory.append(my_item)
self.inventory.append(their_item)
friend.inventory.remove(their_item)
self.inventory.remove(my_item)

Choose a reason for hiding this comment

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

This works! I also see we created helper methods above, the add() and remove() seem useful...

def swap_first_item(self, friend):
if not self.inventory or not friend.inventory:
return False
return self.swap_items(friend, self.inventory[0], friend.inventory[0])

Choose a reason for hiding this comment

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

The efficiency! ✨

)

items = vendor.get_by_category("electronics")
assert len(items) == 0

Choose a reason for hiding this comment

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

Nice!

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