Skip to content

C18 Lions ADSF#102

Open
AlmaDSF wants to merge 10 commits intoAda-C18:masterfrom
AlmaDSF:master
Open

C18 Lions ADSF#102
AlmaDSF wants to merge 10 commits intoAda-C18:masterfrom
AlmaDSF:master

Conversation

@AlmaDSF
Copy link

@AlmaDSF AlmaDSF commented Oct 7, 2022

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.

Great job! Your code overall was good. There are some comments below on places where the code can be improved. For the commits, make sure to commit often and have more descriptive commit messages. Instead of saying what wave was completed, explain what functionality was added or changed. For example, "Created Item class" or "Added swap_items function".

# from pyparsing import condition_as_parse_action
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!

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!

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!


class Item:
pass No newline at end of file
def __init__(self, category = "", condition=None, age = None):

Choose a reason for hiding this comment

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

Condition and age do not need to have None as the default, it can just be 0. We use None for things that are mutable, like lists and dictionaries. Integers are not mutable!

Comment on lines +5 to +6
from operator import ne
from swap_meet.item import Item

Choose a reason for hiding this comment

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

It looks like there were some imports that were accidentally added. It's good practice after you're done coding to make sure you only have the imports you need.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you, I 'll check them.
Actually, VSC added imports very often while I was doing my project.

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 +84 to +85
assert item_c in jesse.inventory
assert item_f in tai.inventory

Choose a reason for hiding this comment

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

Great start! It would be a good idea to check that all of the other items that we expect in the inventory are still in there as well.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you, I'm still working on the project to finish it.

Comment on lines +120 to +124
assert result == True
assert len(jesse.inventory) == 3
assert len(tai.inventory) == 3
assert item_f in tai.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.

Great!

Comment on lines +215 to +216
assert tai.inventory == [item_a, item_b, item_c]
assert jesse.inventory == [item_d, item_e, item_f]

Choose a reason for hiding this comment

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

It would be better to do the same kind of asserts you did in previous tests. == will fail if the lists are out of order. We don't know for sure how the swap works and where the new item will end up in the list. It is better to check if each of the items are in the list.

Comment on lines +251 to +255
assert result == False
assert len(tai.inventory) == 3
assert len(jesse.inventory) == 3
assert jesse.inventory == [item_f, item_e, item_d]
assert tai.inventory == [item_c, item_b, item_a]

Choose a reason for hiding this comment

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

Great!

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