Skip to content

Cedar - Maria O.#97

Open
mobregong wants to merge 2 commits into
Ada-C16:masterfrom
mobregong:master
Open

Cedar - Maria O.#97
mobregong wants to merge 2 commits into
Ada-C16:masterfrom
mobregong:master

Conversation

@mobregong

Copy link
Copy Markdown

No description provided.

@kaidamasaki kaidamasaki left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Waves 1 and 2 look good! 😃

Comment thread swap_meet/vendor.py
Comment on lines +12 to +16
if item in self.inventory:
updated_inventory = self.inventory.remove(item)
else:
item = False
return item

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider refactoring to first check if item is not available. This design is called a guard clause, and can be a nice pattern to follow to enhance readability.

Suggested change
if item in self.inventory:
updated_inventory = self.inventory.remove(item)
else:
item = False
return item
if item not in self.inventory:
return False
updated_inventory = self.inventory.remove(item)
return item

Comment thread swap_meet/vendor.py

def get_by_category(self, category):
items = []
# [items.append(item) for item in self.inventory if item.category == category]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This syntax for a list comprehension will function the same as your classic for loop.

Suggested change
# [items.append(item) for item in self.inventory if item.category == category]
# items = [item in self.inventory if item.category == category]

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.

3 participants