Skip to content

Conversation

@ctlaultdel
Copy link

No description provided.

…r by area, clothing by fabric, electronics by type)
@@ -1,0 +1,8 @@
conditions = {

Choose a reason for hiding this comment

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

Nice use of dict for this! O(1) lookup vs worst case O(n) for n conditions for an if/else to fall through to the last condition!

def check_valid_id(self, id):
if type(id) == int:
return id
elif str(id).isnumeric():

Choose a reason for hiding this comment

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

nice type conversation attempt with error checking!

- (obj) item with id or None
"""
result = list(filter(lambda item: item.id == id, self.inventory))

Choose a reason for hiding this comment

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

nice use of lambda with filter here!


def swap_items(self, other_vendor, my_item, their_item):
"""
Swaps two indicated items between vendors.

Choose a reason for hiding this comment

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

useful, clear comments!

their_item not in other_vendor.inventory):
return False
else:
self.remove(my_item)

Choose a reason for hiding this comment

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

well done making use of existing vendor functions, rather than reimplementing the logic here

:returns:
- (bool) True if swapped or False
"""
[vendor.display_inventory(category) for vendor in [self, other_vendor]]

Choose a reason for hiding this comment

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

nice list comprehension :)


# ~~~~~ swap_by_category_attribute Tests ~~~~~
# ~~~~~ swap decor by area Tests ~~~~~
def test_swap__when_no_matching_area_returns_false():

Choose a reason for hiding this comment

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

nice extra testing :)

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