Skip to content

Ocelots - McKay W#19

Open
mckay1818 wants to merge 8 commits intoada-ac2:mainfrom
mckay1818:main
Open

Ocelots - McKay W#19
mckay1818 wants to merge 8 commits intoada-ac2:mainfrom
mckay1818:main

Conversation

@mckay1818
Copy link
Copy Markdown

No description provided.

Comment thread swap_meet/item.py
if 0 <= self.condition <= 5:
self.condition = float(condition)
else:
raise ValueError("Condition must range from 0-5.")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If you're going to raise an error, I'd do it as early as possible, to avoid executing unnecessary code :)

Comment thread swap_meet/item.py
raise ValueError("Condition must range from 0-5.")

def get_category(self):
return f"{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
item_summary = f"An object of type {self.get_category()} with id {self.id}"
return item_summary

def condition_description(self):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Love the descriptions here :)

Though, you probably now could see that a dict might be useful here: for n conditions to check, a dict will be O(1) lookup, vs worse case O(n) to fall through all of the if/else conditions to the last one.

Comment thread swap_meet/vendor.py
pass No newline at end of file

def __init__(self, inventory=None):
self.inventory = inventory if inventory is not None else []
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 using a ternary operator :)

Comment thread swap_meet/vendor.py
def swap_items(self, other_vendor, my_item, their_item):
if (my_item not in self.inventory or their_item not in other_vendor.inventory):
return False
else:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

small point:

starting a function with if... return can be considered a "guard". There's no need for the else and indentation, as the only way the code executes is if it didn't go down the return path. This can be cleaner, and minimize extra indentation :)

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