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

Sea Turtles- Theresa D. #99

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

perugia33
Copy link

No description provided.


def __str__(self):
return (f"Something to decorate your space.")
Copy link

@tgoslee tgoslee Apr 17, 2022

Choose a reason for hiding this comment

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

you don't have to use a f string because you are just returning the string

return "Something to decorate your space"

Copy link

@tgoslee tgoslee left a comment

Choose a reason for hiding this comment

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

Great job Theresa ! I left some comments on your project on things you did well and suggestions for refactoring. Let me know if you have any questions.



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

Choose a reason for hiding this comment

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

Suggested change
return (f"A gadget full of buttons and secrets.")
return "A gadget full of buttons and secrets."


def __str__(self):
return (f"The finest clothing you could wear.")
Copy link

Choose a reason for hiding this comment

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

Suggested change
return (f"The finest clothing you could wear.")
return "The finest clothing you could wear."

return f"Hello World!"

def condition_description(self):
Copy link

Choose a reason for hiding this comment

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

Good job making an if/elif statement here. I would look into how you could possibly improve this block of code by using other data structures.

Comment on lines +43 to +45
coat_1 = Item(category="clothing")
hat_1 = Item(category="clothing")
Copy link

Choose a reason for hiding this comment

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

remove test code/debugging code when you are finished and before submitting final code.

Comment on lines +37 to +40
self.inventory.remove(my_item)
vendor.inventory.append(my_item)
vendor.inventory.remove(their_item)
Copy link

Choose a reason for hiding this comment

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

you can use the instance methods you already created so self.add(their_item) and self.remove(my_item)

return False

def swap_first_item(self,vendor):
Copy link

Choose a reason for hiding this comment

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

how could you use swap_items() here to reduce redundant code?

Comment on lines +68 to +75
my_best = self.get_best_by_category(their_priority)
if my_best == None:
return False
their_best = other.get_best_by_category(my_priority)
if their_best == None:
return False
return self.swap_items(other, my_best, their_best)
Copy link

Choose a reason for hiding this comment

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

Here is a suggestion for cleaning up your code slightly.

Suggested change
def swap_best_by_category(self, other, my_priority, their_priority):
my_best = self.get_best_by_category(their_priority)
if my_best == None:
return False
their_best = other.get_best_by_category(my_priority)
if their_best == None:
return False
return self.swap_items(other, my_best, their_best)
def swap_best_by_category(self, other, my_priority, their_priority):
my_best = self.get_best_by_category(their_priority)
their_best = other.get_best_by_category(my_priority)
if not my_best or not their_best:
return False
return self.swap_items(other, my_best, their_best)

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