-
Notifications
You must be signed in to change notification settings - Fork 21
swap-meet AC2 - Catherine Bandarchuk #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
be489d6
555b3e9
0d362c1
bbc5ee4
fac82c1
c995b18
95f1d56
4ce08b8
1088c04
60a1a81
2f69e6b
be84c9f
385f4e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,15 @@ | ||
| class Clothing: | ||
| pass | ||
| from .item import Item | ||
|
|
||
| class Clothing( Item ): | ||
|
|
||
| def __init__( self, id = None, condition = None, fabric = None ): | ||
| super().__init__( id, condition ) | ||
|
|
||
| if fabric == None: | ||
| self.fabric = "Unknown" | ||
| else: | ||
| self.fabric = fabric | ||
|
Comment on lines
+8
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another option here could be a python conditional assignment: # Assigns self.fabric to the parameter fabric if it is truthy,
# otherwise assigns self.fabric the value "Unknown"
self.fabric = fabric if fabric else "Unknown" |
||
|
|
||
|
|
||
| def __str__( self ): | ||
| return f"An object of type {super().get_category()} with id {str(self.id)}. It is made from {str(self.fabric)} fabric." | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The superclass base_msg = super().__str__()
return f"{base_msg}. It is made from {str(self.fabric)} fabric." |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,18 @@ | ||
| class Decor: | ||
| pass | ||
| from .item import Item | ||
|
|
||
| class Decor( Item ): | ||
|
|
||
| def __init__( self, id = None, condition = None, width = None, length = None ): | ||
| super().__init__(id, condition) | ||
|
Comment on lines
+3
to
+6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm noticing that in some places there are spaces around keywords or arguments in parentheses. It isn't common to add spaces between parentheses and what goes inside them, to be consistent across the code, I recommend removing them in the future. |
||
| if not width: | ||
| self.width = 0 | ||
| else: | ||
| self.width = width | ||
| if not length: | ||
| self.lenght = 0 | ||
| else: | ||
| self.lenght = length | ||
|
|
||
| def __str__(self): | ||
|
|
||
| return f'''An object of type {super().get_category()} with id {str(self.id)}. It takes up a {str(self.width)} by {str(self.lenght)} sized space.''' | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We would want to break this up over a couple lines to meet the best practice of keeping lines under 79 characters. One option could be to close the quotes and use a return f"An object of type {super().get_category()} with id {str(self.id)}. " \
f"It takes up a {str(self.width)} by {str(self.lenght)} sized space." |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,15 @@ | ||
| class Electronics: | ||
| pass | ||
| from .item import Item | ||
|
|
||
| class Electronics(Item): | ||
|
|
||
| def __init__( self, id = None, condition = None, type = None ): | ||
| super().__init__( id, condition ) | ||
|
|
||
| if not type: | ||
| self.type = "Unknown" | ||
| else: | ||
| self.type = type | ||
|
|
||
|
|
||
| def __str__(self): | ||
| return f"An object of type {super().get_category()} with id {str(self.id)}. This is a {str(self.type)} device." |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,34 @@ | ||
| import uuid | ||
|
|
||
| class Item: | ||
| pass | ||
|
|
||
| def __init__( self, id = None, condition = None ): | ||
| if id == None: | ||
| self.id = uuid.uuid1().int | ||
| else: | ||
| self.id = id | ||
|
|
||
| if condition == None: | ||
| self.condition = 0 | ||
| elif condition > 5 or condition < 0: | ||
| raise IndexError( "Please choose condition in range 0 - 5." ) | ||
| else: | ||
| self.condition = condition | ||
|
|
||
|
|
||
| def get_category( self ): | ||
| return self.__class__.__name__ | ||
|
|
||
|
|
||
| def __str__( self ): | ||
| return f"An object of type {self.get_category()} with id {str(self.id)}" | ||
|
|
||
|
|
||
| def condition_description( self ): | ||
| condition_list = ["Poor Condition", # condition = 0 | ||
| "Heavily Used", # condition = 1 | ||
| "Used - Good Condition", # condition = 2 | ||
| "Used - Very Good Condition", # condition = 3 | ||
| "Used - Great Condition", # condition = 4 | ||
| "New"] # condition = 5 | ||
| return condition_list[self.condition] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice handling for condition description! It looks like the code assumes that |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,202 @@ | ||
| class Vendor: | ||
| pass | ||
|
|
||
| def __init__( self, inventory = None ): | ||
| if inventory == None: | ||
| self.inventory = [] | ||
| else: | ||
| self.inventory = inventory | ||
|
|
||
|
|
||
|
Comment on lines
+8
to
+9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is minor: the PEP8 style guide recommends a single blank line to separate methods inside of a class. |
||
| def add( self, item ): | ||
| self.inventory.append( item ) | ||
| return item | ||
|
|
||
|
|
||
| def remove( self, item ): | ||
| if self.inventory == None or item not in self.inventory: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In python, when we're comparing an object to |
||
| return None | ||
|
|
||
| self.inventory.remove( item ) | ||
| return item | ||
|
|
||
|
|
||
| def get_by_id (self, item_id ): | ||
| if not self.inventory: | ||
| return None | ||
|
|
||
| for item in self.inventory: | ||
| if item.id == item_id: | ||
| return item | ||
|
|
||
| return None | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like the explicit return for None if the item can't be found. |
||
|
|
||
|
|
||
| def swap_items( self, other_vendor, my_item, their_item ): | ||
| if not self.inventory or \ | ||
| not other_vendor.inventory or \ | ||
| not my_item or not their_item or \ | ||
| my_item not in self.inventory or \ | ||
| their_item not in other_vendor.inventory: | ||
| return False | ||
|
Comment on lines
+35
to
+40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To make it clear where the if-statement ends and the indented body of the block begins, I generally suggest using double indentation on the broken up lines. To make the operators more prominent I also suggest putting all operators that apply to a following statement on the same line as the statement: if not self.inventory \
or not other_vendor.inventory \
or not my_item or not their_item \
or my_item not in self.inventory \
or their_item not in other_vendor.inventory:
return False |
||
|
|
||
| self.add( their_item ) | ||
| other_vendor.add( my_item ) | ||
| self.remove( my_item ) | ||
|
Comment on lines
+43
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice use of the Vendor methods 🙂 |
||
| other_vendor.remove( their_item ) | ||
|
|
||
| return True | ||
|
|
||
|
|
||
| def swap_first_item( self, other_vendor ): | ||
| if not self.inventory or \ | ||
| not other_vendor.inventory: | ||
| return False | ||
|
|
||
| return self.swap_items( other_vendor, self.inventory[0], other_vendor.inventory[0] ) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Something to consider about using self.inventory[0], other_vendor.inventory[0] = other_vendor.inventory[0], self.inventory[0] |
||
|
|
||
|
|
||
| def get_by_category( self, category = None ): | ||
| list_of_objects = [] | ||
|
|
||
| if not self.inventory or category == None: | ||
| return [] | ||
|
|
||
| for item in self.inventory: | ||
| if item.get_category() == str(category): | ||
| list_of_objects.append( item ) | ||
|
|
||
| return list_of_objects | ||
|
Comment on lines
+59
to
+68
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Notice this pattern of: result_list = []
for element in source_list:
if some_condition(element):
result_list.append(element)can be rewritten using a list comprehension as: result_list = [element for element in source_list if some_condition(element)]Which here would look like: list_of_objects = [item for item in self.inventory if item.get_category() == category]At first, this may seem more difficult to read, but comprehensions are a very common python style, so I encourage y’all to practice working with them! |
||
|
|
||
|
|
||
| def get_best_by_category( self, category ): | ||
| list_of_objects = self.get_by_category(category) | ||
|
|
||
| if not list_of_objects: | ||
| return None | ||
|
|
||
| best_item = list_of_objects[0] | ||
| for item in list_of_objects: | ||
| if item.condition > best_item.condition: | ||
| best_item = item | ||
|
|
||
| return best_item | ||
|
Comment on lines
+77
to
+82
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice loop! The time complexity would be the same, but another way we could write this is using the return max(list_of_objects, key=lambda item: item.condition) |
||
|
|
||
|
|
||
| def swap_best_by_category( self, other_vendor, my_priority, their_priority ): | ||
|
|
||
| best_item_for_them = self.get_best_by_category(their_priority) | ||
| best_item_for_me = other_vendor.get_best_by_category(my_priority) | ||
|
|
||
| return self.swap_items(other_vendor, best_item_for_them, best_item_for_me) | ||
|
Comment on lines
+87
to
+90
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like that since you have such strong validation in |
||
|
|
||
|
|
||
| def display_inventory(self, category = None): | ||
| if not self.inventory: | ||
| print("No inventory to display.") | ||
| return None | ||
|
|
||
| if category == None: | ||
|
|
||
| for index in range( len( self.inventory )): | ||
| print(f"{index+1}. {self.inventory[index]}") | ||
|
Comment on lines
+100
to
+101
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice use of for index, item in enumerate(self.inventory, start=1):
print(f"{index}. {item}") |
||
|
|
||
| else: | ||
| count = 0 | ||
| for item in self.inventory: | ||
| if item.get_category() == category: | ||
| count += 1 | ||
| print(f"{count}. {item}") | ||
|
Comment on lines
+104
to
+108
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we weren't worried about how large these lists could be, we could reduce some similar code by calling items_to_display = self.inventory
if category:
items_to_display = self.get_by_category(category)
if not items_to_display:
print("No inventory to display.")
return None
for index, value in enumerate(self.inventory, start=1):
print(f"{index}. {item.__str__()}") |
||
|
|
||
| if count == 0: | ||
| print("No inventory to display.") | ||
| return None | ||
|
|
||
|
|
||
| def swap_by_id( self, other_vendor, my_item_id, their_item_id ): | ||
| my_item = self.get_by_id( my_item_id ) | ||
| their_item = other_vendor.get_by_id( their_item_id ) | ||
|
|
||
| if not my_item or not their_item: | ||
| return False | ||
|
|
||
| else: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could remove the |
||
| return self.swap_items(other_vendor, my_item, their_item) | ||
|
|
||
| def choose_and_swap_items( self, other_vendor, category = None ): | ||
| if category == None: | ||
| self.category = "" | ||
| else: | ||
| self.category = category | ||
|
|
||
| self.display_inventory( self.category ) | ||
| other_vendor.display_inventory( self.category ) | ||
|
|
||
| my_item_id = int( input( "Please enter the id of the item from my inventory:" )) | ||
| their_item_id = int( input( "Please enter the id of the item from their inventory:" )) | ||
|
|
||
| if self.swap_by_id( other_vendor, my_item_id, their_item_id ): | ||
| return True | ||
| return False | ||
|
Comment on lines
+137
to
+139
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could remove the if/else by directly returning the result of |
||
|
|
||
| # Function swap first two clothing with the same attribute fabric | ||
| # If we want to swap all clothing with the same fabric | ||
| # we can use next function : swap_all_clothing_by_attributes( self, other_vendor ) | ||
| def swap_clothing_by_attributes( self, other_vendor ): | ||
| list_of_my_clothing = self.get_by_category( "Clothing" ) | ||
| list_of_their_clothing = other_vendor.get_by_category( "Clothing" ) | ||
|
|
||
| for my_item in list_of_my_clothing: | ||
| for their_item in list_of_their_clothing: | ||
|
|
||
| if my_item.fabric == their_item.fabric: | ||
| self.swap_by_id( other_vendor, my_item.id, their_item.id ) | ||
| return True | ||
|
|
||
| return False | ||
|
|
||
| # Function swap all clothing with the same attribute fabric | ||
| def swap_all_clothing_by_attributes( self, other_vendor ): | ||
| list_of_my_clothing = self.get_by_category( "Clothing" ) | ||
| list_of_their_clothing = other_vendor.get_by_category( "Clothing" ) | ||
| counter = 0 | ||
|
|
||
| for my_item in list_of_my_clothing: | ||
| for their_item in list_of_their_clothing: | ||
|
|
||
| if my_item.fabric == their_item.fabric: | ||
| counter +=1 | ||
| self.swap_by_id( other_vendor, my_item.id, their_item.id ) | ||
|
|
||
| if counter > 0: | ||
| return True | ||
|
|
||
| return False | ||
|
|
||
|
|
||
| def swap_decor_by_attributes( self, other_vendor ): | ||
| list_of_my_decor = self.get_by_category( "Decor" ) | ||
| list_of_their_decor = other_vendor.get_by_category( "Decor" ) | ||
|
|
||
| for my_item in list_of_my_decor: | ||
| for their_item in list_of_their_decor: | ||
|
|
||
| if my_item.width == their_item.width \ | ||
| and my_item.lenght == their_item.lenght \ | ||
| or my_item.width * my_item.lenght == their_item.width * their_item.lenght: | ||
| self.swap_by_id( other_vendor, my_item.id, their_item.id ) | ||
| return True | ||
|
|
||
| return False | ||
|
|
||
| def swap_electronics_by_attributes( self, other_vendor ): | ||
| list_of_my_electronics = self.get_by_category( "Electronics" ) | ||
| list_of_their_electronics = other_vendor.get_by_category( "Electronics" ) | ||
|
|
||
| for my_item in list_of_my_electronics: | ||
| for their_item in list_of_their_electronics: | ||
|
|
||
| if my_item.type == their_item.type: | ||
| self.swap_by_id( other_vendor, my_item.id, their_item.id ) | ||
| return True | ||
|
|
||
| return False | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strings are an immutable type; unlike lists and dictionaries, we could set the default in the function definition to
"Unknown", which could let us remove the if-check.