Skip to content
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

Maple- Sabrina L. #94

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ When our test failures leave us confused and stuck, let's use the detailed proje

The first two tests in wave 1 imply:

- There is a module (file) named `vendor.py` inside of the `swap_meet` package (folder)
- There is a module (file) named `vendor.py` inside of the `swap_meet` package (folder) -->
- Inside this module, there is a class named `Vendor`
- Each `Vendor` will have an attribute named `inventory`, which is an empty list by default
- When we create initialize an instance of `Vendor`, we can optionally pass in a list with the keyword argument `inventory`
Expand All @@ -192,10 +192,10 @@ The first tests in wave 2 imply:

- Inside this module, there is a class named `Item`
- Each `Item` will have an attribute named `category`, which is an empty string by default
- When we initialize an instance of `Item`, we can optionally pass in a string with the keyword argument `category`
- When we initialize an instance of `Item`, we can optionally pass in a string with the keyword argument `category` -->
- Instances of `Vendor` have an instance method named `get_by_category`
- It takes one argument: a string, representing a category
- This method returns a list of `Item`s in the inventory with that category
- This method returns a list of `Item`s in the inventory with that category -->

### Wave 3

Expand Down Expand Up @@ -233,7 +233,7 @@ The tests in wave 4 imply:
The tests in Wave 5 imply there are three additional modules with three additional classes:

- `Clothing`
- Has an attribute `category` that is `"Clothing"`
- Has an attribute `category` that is `"Clothing"` -->
- Its stringify method returns `"The finest clothing you could wear."`
- `Decor`
- Has an attribute `category` that is `"Decor"`
Expand Down
12 changes: 10 additions & 2 deletions swap_meet/clothing.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,10 @@
class Clothing:
pass
from swap_meet.item import Item

class Clothing(Item):
def __init__(self, condition = 0):
# super().__init__(category = "Clothing")
## super().__init__(condition)
self.category = "Clothing"
self.condition = condition
Comment on lines +5 to +8

Choose a reason for hiding this comment

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

great! lines 7 and 8 totally work, though we could definitely use the parent class' constructor instead! It already creates category and condition attributes for us, so let's look at that:

Suggested change
# super().__init__(category = "Clothing")
## super().__init__(condition)
self.category = "Clothing"
self.condition = condition
# super().__init__(category = "Clothing", condition)

and I like that you hardcoded "Clothing" since of course this child class Clothing will always be category "Clothing." No reason to give the user the opportunity to mess that up haha

def __str__(self):
return "The finest clothing you could wear."
12 changes: 10 additions & 2 deletions swap_meet/decor.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,10 @@
class Decor:
pass
from swap_meet.item import Item

class Decor(Item):

def __init__(self, condition = 0):
self.category = "Decor"
self.condition = condition
Comment on lines +4 to +7

Choose a reason for hiding this comment

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

same as above! we could refactor this a little more to use inheritance more explicitly


def __str__(self):
return "Something to decorate your space."
12 changes: 10 additions & 2 deletions swap_meet/electronics.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,10 @@
class Electronics:
pass
from swap_meet.item import Item

class Electronics(Item):

Choose a reason for hiding this comment

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

👍

def __init__(self, condition = 0):
self.category = "Electronics"
self.condition = condition

def __str__(self):
return "A gadget full of buttons and secrets."

18 changes: 17 additions & 1 deletion swap_meet/item.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,18 @@
class Item:

Choose a reason for hiding this comment

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

👍

pass
def __init__(self, category = ""):
self.category = category
def __str__(self):
return "Hello World!"
def condition_description(self):
if self.condition < 1:
return "poor"
elif self.condition < 2:
return "bad"
elif self.condition < 3:
return "okay"
elif self.condition < 4:
return "decent"
elif self.condition < 5:
return "nearly new"
else:
return "this is so bad it does not have a condition description"
57 changes: 56 additions & 1 deletion swap_meet/vendor.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,57 @@
from swap_meet.item import Item

Choose a reason for hiding this comment

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

so here, we don't actually need to import the literal class Item itself. We aren't instantiating an instance of Item (ie, Item(category="antique",3) ) inside the Vendor anywhere.

There are "items" coming in through parameters, but the class "blueprint" itself can't tell that. On line 7 def add(self, item) the item parameter is simply a placeholder. It could be called banana, apple, and when this method is actually called we could pass an integer, a string, a dictionary, etc. basically, the method itself doesn't know what these parameter values will be.

So unless we are actually creating an instance inside vendor (like our composition examples), we don't need to import it


class Vendor:
pass
def __init__(self, inventory = []):
self.inventory = inventory
Comment on lines +4 to +5

Choose a reason for hiding this comment

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

careful passing in a data structure like a list as a default parameter in a class constructor. Every instance that is made will all use the same list instead of each instance having its own unique inventory list. this is known as a "side effect."

so what we can do to stop this side effect is:

Suggested change
def __init__(self, inventory = []):
self.inventory = inventory
def __init__(self, inventory = None):
if inventory == None:
self.inventory = []
else:
self.inventory = inventory

now for every instance of Vendor that is created without a pre-made inventory list, the instance will get its own unique empty list.


def add(self, item):

Choose a reason for hiding this comment

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

👍

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.

👍

if item in self.inventory:
self.inventory.remove(item)
return item
else:
return False

def get_by_category(self, category):

Choose a reason for hiding this comment

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

👍

inventory_list = []
for item in self.inventory:
if item.category == category:
inventory_list.append(item)
return inventory_list

def swap_items(self, friend, my_item, their_item):
if their_item in friend.inventory and my_item in self.inventory:
self.add(their_item)
friend.add(my_item)
self.remove(my_item)
friend.remove(their_item)
Comment on lines +27 to +30

Choose a reason for hiding this comment

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

great job reusing methods you've already made!we can make this even shorter, though:

Suggested change
self.add(their_item)
friend.add(my_item)
self.remove(my_item)
friend.remove(their_item)
self.add(friend.remove(their_item))
friend.add(self.remove(my_item))

if you recall, the remove method returns the item that is removed, so we can call the remove method right inside the add

return True
else:
return False

def swap_first_item(self, friend):

Choose a reason for hiding this comment

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

👍

if self.inventory and friend.inventory:
self.swap_items(friend, self.inventory[0], friend.inventory[0])
return True
return False

def get_best_by_category(self, category):

Choose a reason for hiding this comment

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

👍

items = self.get_by_category(category)
if not items:
return None
best = items[0]
for item in items:
if item.condition > best.condition:
best = item
return best


def swap_best_by_category(self, other, my_priority, their_priority):

Choose a reason for hiding this comment

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

👍

my_best = self.get_best_by_category(their_priority)
their_best = other.get_best_by_category(my_priority)
if not my_best and their_best:
return False
return self.swap_items(other,my_best, their_best)