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

Ruby - Mikayla H #84

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Ruby - Mikayla H #84

wants to merge 10 commits into from

Conversation

MDHowden2020
Copy link

No description provided.

Copy link

@nancy-harris nancy-harris left a comment

Choose a reason for hiding this comment

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

Excellent job! There are some comments below on places to make the code cleaner/simpler. Great job on commits! It would be great if the commits could give more details about what changes were made. Something like "Created the Item class" would be great!

from swap_meet.item import Item

class Clothing(Item):

Choose a reason for hiding this comment

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

Excellent!

Comment on lines +16 to +17
return super().condition_description()

Choose a reason for hiding this comment

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

If the only thing you're doing is calling the super, then you don't need to override the method! If Python sees that the Clothing class does not have a condition_description method, it will look in the Item class, see it there, and use that one. Try removing this method from Clothing (and Decor and Electronics) and you'll see that all of the tests still pass!

from swap_meet.item import Item

class Decor(Item):

Choose a reason for hiding this comment

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

Excellent!

pass
from swap_meet.item import Item

class Electronics(Item):

Choose a reason for hiding this comment

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

Excellent!

Comment on lines +10 to +11
return "Clothing"

Choose a reason for hiding this comment

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

This method is not necessary! The .__class__.__name__ attribute that's used in the Item class will adjust and print the correct thing. Try deleting this method in the Clothing class (and Decor and Electronics) and the tests will still pass!

@@ -33,12 +34,12 @@ def test_get_no_matching_items_by_category():

items = vendor.get_by_category("Electronics")

raise Exception("Complete this test according to comments below.")
assert len(items) == 0

Choose a reason for hiding this comment

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

Excellent!

Comment on lines +114 to +122
assert result
assert len(tai.inventory) == 3
assert len(jesse.inventory) == 3
assert item_f in tai.inventory
assert item_a in tai.inventory
assert item_b in tai.inventory
assert item_d in jesse.inventory
assert item_e in jesse.inventory
assert item_c in jesse.inventory

Choose a reason for hiding this comment

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

Excellent!

Comment on lines +156 to +166
assert result
assert len(tai.inventory) == 3
assert len(jesse.inventory) == 3
assert item_a in tai.inventory
assert item_b in tai.inventory
assert item_f in tai.inventory
assert item_d in jesse.inventory
assert item_e in jesse.inventory
assert item_c in jesse.inventory
assert item_f not in jesse.inventory
assert item_c not in tai.inventory

Choose a reason for hiding this comment

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

Excellent!

Comment on lines +251 to +259
assert not result
assert len(tai.inventory) == 3
assert len(jesse.inventory) == 3
assert item_a in tai.inventory
assert item_b in tai.inventory
assert item_c in tai.inventory
assert item_d in jesse.inventory
assert item_e in jesse.inventory
assert item_f in jesse.inventory

Choose a reason for hiding this comment

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

Excellent!

Comment on lines +293 to +301
assert not result
assert len(tai.inventory) == 3
assert len(jesse.inventory) == 3
assert item_a in tai.inventory
assert item_b in tai.inventory
assert item_c in tai.inventory
assert item_d in jesse.inventory
assert item_e in jesse.inventory
assert item_f in jesse.inventory

Choose a reason for hiding this comment

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

Excellent!

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