Skip to content

Ocelots - Diana Arreola#7

Open
diarreola wants to merge 24 commits intoada-ac2:mainfrom
diarreola:main
Open

Ocelots - Diana Arreola#7
diarreola wants to merge 24 commits intoada-ac2:mainfrom
diarreola:main

Conversation

@diarreola
Copy link
Copy Markdown

:)

@diarreola diarreola changed the title Swap Meet submission Ocelots - Diana Arreola Dec 8, 2022
Comment thread swap_meet/item.py
class Item:
pass No newline at end of file
def __init__(self, id=None, condition=0):
self.id = uuid.uuid1().int if not id else id
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nice example of a ternary operator

Comment thread swap_meet/item.py
self.condition = condition

def get_category(self):
return self.__class__.__name__
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nice use of python introspection

Comment thread swap_meet/item.py
return f"An object of type {self.get_category()} with id {self.id}"

def condition_description(self):
descriptions = {0: "torn", 1: "a bit shredded", 2: "defs used", 3: "lightly used", 4: "near perfect", 5: "perf",}
Copy link
Copy Markdown

@ilana-adadev ilana-adadev Dec 23, 2022

Choose a reason for hiding this comment

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

minor suggestion: formatting this dictionary on multiple lines for readably

Comment thread swap_meet/item.py
return f"An object of type {self.get_category()} with id {self.id}"

def condition_description(self):
descriptions = {0: "torn", 1: "a bit shredded", 2: "defs used", 3: "lightly used", 4: "near perfect", 5: "perf",}
Copy link
Copy Markdown

@ilana-adadev ilana-adadev Dec 23, 2022

Choose a reason for hiding this comment

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

nice use of a dict to store this info, vs an if statement. You can imagine that a long if-else statement would take worst case O(N) to fall through to the last condition. ;)

Comment thread swap_meet/vendor.py
return None

def swap_items(self, other_vendor, my_item, their_item):
if my_item == their_item:
Copy link
Copy Markdown

@ilana-adadev ilana-adadev Dec 23, 2022

Choose a reason for hiding this comment

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

nice use of guard statements to return as fast as possible before executing other code; I appreciate that you didn't indent the rest of the function as an else code block; no need as the code doesn't continue past the returns. Yours is the cleaner style.

Comment thread swap_meet/vendor.py
if not self.inventory:
return []

item_category_list = list(filter(lambda item: item.get_category() == category, self.inventory))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nice lambda!

Comment thread swap_meet/vendor.py
if not item_category_list:
return None

max_item = max(item_category_list, key=lambda item: item.condition)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ahhhh well done, nice to see max used here!

Comment thread swap_meet/vendor.py
their_priority_item_in_my_inventory = self.get_best_by_category(their_priority)
my_priority_item_in_their_inventory = other_vendor.get_best_by_category(my_priority)
if my_priority_item_in_their_inventory == None or their_priority_item_in_my_inventory == None:
False
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@diarreola confused about this False on a standalone line. return False?

Copy link
Copy Markdown
Author

@diarreola diarreola Dec 23, 2022

Choose a reason for hiding this comment

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

me too 🤔, I think I did mean to have return False

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nodding. Thank you for reviewing the feedback and following up! I appreciate that!

Comment thread swap_meet/vendor.py
return

print_inventory(category_based_inventory)
return
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

no need for this return at the end of the function :)

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