Sapphire- Janice N

[janice15]

No description provided.

[tildeee]

Good work on this project, Janice! Because of the bugs and failing tests around the category features, this project is marked as a "yellow," asking you to review the project.

Overall, the features you've implemented are well-done; it's clean, straightforward, good code. Your test implementations look great. Your overall code style looks good. Keep it up!

My feedback is largely around some logical bugs I see around how you check the category in each item. In addition, I see a lot of opportunities in your classes to practice Inheritance.

Your git hygiene also looks pretty good! Keep up the frequency and quality of commit messages. This is a small "nitpicky" piece of feedback, but I think your commit messages would be better if they used the word "implemented" instead of "completed"-- simply because code is never "completed," so it feels weird to me to see a commit that claims it's completed lol :)

I'd encourage you to check your understanding/check the feedback on these concepts:

• Instance methods
• Inheritance
• Debugging/confirming what your code is doing

Please let me know any and all questions that come up, especially if they're about those above three concepts!

Finally, in this project submission, on my end it looks like the tests for waves 2, 4, and 5 were still skipped.

Again, good work on this project. I want to make sure you can apply OOP principles to every project going forward, so let me know what questions you have or would like to review.

[tildeee]

Here, you're creating an instance variable on Vendor named item, and assigning it to the passed in item... meaning you're expecting every instance of Vendor (for example, instance_of_vendor = Vendor()) to store one single item, which we can access with instance_of_vendor.item

In this situation, we don't have a requirement that Vendors should have an item. Also, contextually, I'm not sure it makes sense that Vendors have one item... Especially since in this project they already have an inventory, which contains multiple items.

This is all to say, I think we can delete the line self.item = item!

[tildeee]

Similar to above, we don't ever access the instance variable item (only the passed-in argument item)... so we can delete this line, too.

[tildeee]

These lines look suspiciously familiar to other code in this file! This is a good opportunity to use the other instance methods add and remove that are defined in Vendor. Consider this refactoring, which also passes the tests:

        self.remove(my_item)
        self.add(their_item)
        other_vendor.add(my_item)
        other_vendor.remove(their_item)

[tildeee]

Nice use of swap_items!

[tildeee]

I'm not sure this code does what you want it to do! self.inventory will always be a list of Item instances, whereas category will always be a string (such as "Clothing".) In most cases, category will not be in self.inventory.

Consider this Python snippet to demonstrate that:

from clothing import Clothing # Depending on where we write this test code snippet, we'll need to make sure we import/define Clothing

inventory = [Clothing(), Clothing(), Clothing()]
print("The contents of inventory are:", inventory)
category = "Clothing"
if category in inventory:
    print("Category Clothing is in this inventory of 3 Clothing instances") # Your current code logic says that this should be True if there's at least one Clothing item in the inventory.
else:
    print("Category Clothing is not in inventory of 3 Clothing instances")

When we run this, we'll find that the else branch prints, meaning that the similar if category not in self.inventory in your code will also behave similarly.

How do we find items in the inventory that have a category of "Clothing"? How do we look at each item and find its category? Your logic in get_by_category that uses Item's get_category instance method is the way to go here.

[tildeee]

In the requirements for swap_best_by_category, the value of my_priority and their_priority is a string that's a name of a category. In this code here, you're assigning my_item to this string, and then using it in swap_items (which expects an Item instance). Before we call swap_items, we'll need to accurately find the value for my_item and their_item.

[tildeee]

Here, you're iterating through descriptions and printing the value of the keys matching self.condition. Our requirements for condition_description are to return it, not print it.

[tildeee]

Clothing should inherit from Item! Once it inherits from Item, we can rely on inheritance. We can delete the redundant definitions of get_category and condition_description in Clothing, so each instance of Clothing inherits get_category and condition_description from Item instead.

[tildeee]

Our project requirements state that condition_description should have the same behavior between Clothing, Decor, and Electronics. While having three identical implementations certainly works, these three inherit from Item... it would be good to take advantage of that, here!

If we delete condition_description here, then Electronics will look to its superclass (Item) for a definition of condition_description.

[tildeee]

My comments about inheritance apply here, too.

However, I need to point out-- this method name is mispelled, and should be condition_description, not condition_desciption.