Skip to content

Cheetahs - Ev#104

Open
evelynburrito wants to merge 6 commits intoAda-C18:masterfrom
evelynburrito:master
Open

Cheetahs - Ev#104
evelynburrito wants to merge 6 commits intoAda-C18:masterfrom
evelynburrito:master

Conversation

@evelynburrito
Copy link

No description provided.

@evelynburrito evelynburrito changed the title Swap Meet Cheetahs - Ev Oct 7, 2022
Copy link

@nancy-harris nancy-harris left a comment

Choose a reason for hiding this comment

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

Great start! You've got a lot of good functionality! I left comments below on some things that would make the code simpler/easier. Since at least one wave is not completed, this project is yellow.

For commits, make sure to commit often! You're off to a good start, but some of the commits could be smaller.

pass No newline at end of file
from .item import Item

class Clothing(Item):

Choose a reason for hiding this comment

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

Great start on using the Item super class here! However, this doesn't fully take advantage of what super classes can do! You don't need to story category or condition information here, you can pass that up to the Item class. None of the child classes need those lines of code, which will make the code cleaner.

@@ -1,2 +1,23 @@
class Item:
pass No newline at end of file
def __init__(self, category = None, condition = 0):

Choose a reason for hiding this comment

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

Category can be set to "" as a default. Strings are immutable in Python, so you don't need to use None. None is for when something is mutable (like a list or a dictionary).


class Vendor:
pass No newline at end of file
def __init__(self, inventory = None):

Choose a reason for hiding this comment

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

Excellent!

inventory = []
self.inventory = inventory

def add(self, item):

Choose a reason for hiding this comment

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

Excellent!

self.inventory.append(item)
return item

def remove(self, item):

Choose a reason for hiding this comment

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

Excellent!

return False

def get_by_category(self, category):
self.category = category

Choose a reason for hiding this comment

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

There's no need to set self.category here. Vendors shouldn't have a category!

Comment on lines +32 to +33
self.inventory.remove(my_item)
vendor.inventory.append(my_item)

Choose a reason for hiding this comment

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

These would've been good places to use your own add and remove functions that you created earlier.

Comment on lines +44 to +48
vendor.inventory.append(self.inventory[0])
self.inventory.remove(self.inventory[0])

self.inventory.append(vendor.inventory[0])
vendor.inventory.remove(vendor.inventory[0])

Choose a reason for hiding this comment

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

Swapping items sounds a lot like what the swap_items function does above. You could simplify this code by calling your own function.


raise Exception("Complete this test according to comments below.")
# *********************************************************************
assert result == False

Choose a reason for hiding this comment

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

It would also be good to add an assert to make sure the length of the inventory didn't get changed.

items = vendor.get_by_category("electronics")

raise Exception("Complete this test according to comments below.")
assert len(items) == 0

Choose a reason for hiding this comment

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

Excellent!

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