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

C19 Sapphire - Linh Huynh #36

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

C19 Sapphire - Linh Huynh #36

wants to merge 29 commits into from

Conversation

mlhuynh
Copy link

@mlhuynh mlhuynh commented Apr 7, 2023

Thank you for reviewing my code. :)

…ote code for instance methods named 'add' and 'remove'.
…hat are not in the inventory. Completed Wave 1.
…_wave_01.py. Also removed exception in last subtest and deleted @pytest.mark.skip lines before each subtest.
…lse function of unit test_wave_03.py module. Also removed 'raise Exception' statement and all @pytest.mark.skip statments before each subtest/function.
…test_wave_03.py module. Re-pushing with the saved changes.
…ethods for Clothing, Decor, and Electronics classes respectively in the clothing.py, decor.py, and electronics.py modules.
…onics classes respectively in the item.py, clothing.py, decor.py, and electronics.py modules.
…ethods in Item class of item.py. Also updated the Clothing, Decor, and Electronic classes to include the attribute named condition in super().
…ve_06.py module: test_get_no_matching_items_by_category, test_swap_best_by_category, test_swap_best_by_category_reordered, test_swap_best_by_category_no_match_is_false, and test_swap_best_by_category_no_other_match_is_false. Also removed all skip statements before each function.
Copy link
Contributor

@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, Linh! It's marked as a "green" on my end.

Overall, your code looks great. Your code is well-written, readable, logical, straightforward, and clean. It's clear to me that you've put in effort to refactor. I wish I had more specific feedback for you, but really, overall I think you're writing good code and thinking about the right things! Keep it up!

In addition, your git hygiene looks great, and your tests look great, too!

As a small note, it looks like your swap-meet repo is a fork of AdaGold/swap-meet, instead of the expected Ada-C19/swap-meet. There's nothing to fix here for now, but I just thought I'd point that out to you.

Again, good work! Let me know what questions come up.

def __init__(self, inventory = None):
self.inventory = [] if inventory is None else inventory
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice post-fix conditional!

try:
return self.swap_items(other_vendor, self.inventory[0], other_vendor.inventory[0])
except IndexError:
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice IndexError catch! I like how the code looks here.


def get_by_category(self, category):
return [item for item in self.inventory if item.get_category() == category]
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!!

2: "As-Is: Acceptable but of course, that's subjective. Consider at your own discretion.",
3: "Good: Functional or wearable. What more could you ask for a discounted price?",
4: "Gently worn: Practically like new. It's basically a steal. At least it's not cursed.",
Copy link
Contributor

Choose a reason for hiding this comment

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

👻

5: "New: Pretty much in pristine condition with the added bonus of preventing existential crises and attracting good luck."
}
return condition_rating[self.condition]
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the use of the dictionary/map

Comment on lines +8 to +10
# if I removed this instance method altogether, referenced the __str__ method instance in the Item class,
# and edited the return string in __str__ to remove any redundancy.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a good decision for this project/context!

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.

3 participants