Skip to content

Intesar C18 lions#91

Open
Intesarsiraj wants to merge 5 commits intoAda-C18:masterfrom
Intesarsiraj:master
Open

Intesar C18 lions#91
Intesarsiraj wants to merge 5 commits intoAda-C18:masterfrom
Intesarsiraj:master

Conversation

@Intesarsiraj
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! Your code overall looks really good! There are some places where there are a lot of empty lines between code. Make sure to remove empty lines that are not needed.

Remember to add commits more often and to add commit messages that are more descriptive. Instead of saying which wave you're completed, add messages like "Added Clothing class" or "Added get_best_by_category function".

class Clothing:
pass No newline at end of file
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!

class Decor:
pass No newline at end of file
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!

class Electronics:
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!

@@ -1,2 +1,27 @@
class Item:
pass No newline at end of file
def __init__(self,category = "", condition = 0.0):

Choose a reason for hiding this comment

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

Excellent!

@@ -1,2 +1,88 @@
class Vendor:
pass No newline at end of file
def __init__(self, inventory = None):

Choose a reason for hiding this comment

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

Excellent!


result = vendor.remove(item)

assert not result

Choose a reason for hiding this comment

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

Great assert! It would also be a good idea to check that the inventory length is still 3 and we didn't accidentally remove something.

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 +81 to +82
assert len(jesse.get_by_category("Clothing")) == 1
assert len(jesse.get_by_category("Decor")) == 2

Choose a reason for hiding this comment

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

Good start, but these asserts only look at Jesse's inventory. Also, having the correct number of categories of items does not necessarily mean we have the specific items we are expecting. Instead, it would be good to do something like what you do for test_swap_best_by_category_reordered() test

their_priority="Clothing"
)

assert not result

Choose a reason for hiding this comment

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

Good start! It would also be a good idea to check that the items in the inventories did not change, like you do for the other test.

Comment on lines +246 to +254
assert result == False
assert len(jesse.inventory) == 3
assert len(tai.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