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

Sapphire - Sophia Tran #71

Open
wants to merge 39 commits into
base: main
Choose a base branch
from
Open

Sapphire - Sophia Tran #71

wants to merge 39 commits into from

Conversation

sophiat8832
Copy link

No description provided.

Copy link

@tildeee tildeee left a comment

Choose a reason for hiding this comment

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

Great work on this project, Sophia! This project is marked as a "green" on my end.

Overall, your code looks great, and your solutions look like what I was looking for. Your code is readable and efficient. Your code demonstrates good understanding of OOP, instance methods, and inheritance. I wish I had more specific feedback for you, but genuinely, you hit all of the requirements and your approaches are good... I don't have much to say besides keep doing what you're doing!

In addition, your git hygiene looks great! Also, your test implementations look good too.

On a small note, in your project submission, on my end it looks like the tests for waves 2, 4, and 5 are still skipped.

That being said, great work. Please let me know what questions come up, and keep up the good job.

Comment on lines +29 to +33
other_vendor.add(my_item)

other_vendor.remove(their_item)
self.add(their_item)
Copy link

Choose a reason for hiding this comment

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

Nice work using add and remove!

return False

self.swap_items(other_vendor, self.inventory[0], other_vendor.inventory[0])
Copy link

Choose a reason for hiding this comment

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

Nice work using swap_items!

Comment on lines +54 to +61
condition = 0

for item in self.inventory:
if item.get_category() == category and item.condition > condition:
condition = item.condition
best_item = item
return best_item
Copy link

Choose a reason for hiding this comment

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

Your solution works, and it's a good solution especially since it only loops through inventory once! Optionally, there's a way to refactor this so it uses the get_by_category function and then calls max, if you wanted to practice that approach.

3: "Okay",
4: "Pretty good",
5: "Brilliant, incredible, amazing, show stopping, spectacular, never the same, totally unique, completely not ever been done before"
Copy link

Choose a reason for hiding this comment

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

💥 💥 💥 💥 💥 💥

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