Sharks - Ivana M.

[maldonado-ivana]

No description provided.

[yangashley]

Nice job, Ivana!

All required waves are complete and all required tests are passing! Your code is well organized and you used descriptive variable names. Your general approaches are good, and you're off to a good start with OOP.

I called out some places where you can reverse your logic so you implement a guard clause instead of if/else. This will make your python code more idiomatic. Avoiding unnecessary indenting (by using guard clauses), and checking conditions making use of pythons ideas about truthy and falsy can communicate your intent to other python developers very quickly.

I also pointed out where you can use list comprehension and list destructuring if you're up for it.

There was place that you could leverage the return values from other methods to make your method a little more concise.

Previously wrote something about your check_condition function but I just went back and reread it more closely and it works! So I deleted the comment. It looks good!

Overall, well done 🟢 !

[yangashley]

👍

Since you have the category and condition parameters in the correct order when you pass it to the parent's constructor, you can also write line 5 like:
super().__init__("Clothing", condition)

[yangashley]

I think this is a stray import that can be removed since you don't use pytest in Electronics

[yangashley]

👍

This can be re-written as a ternary, as:

my_var = "some value" if some_condition else "other value"

In this situation, that would look like

self.inventory = [] if inventory is None else inventory

[yangashley]

A more explicit way to return False would be to use a guard clause instead doing it in the else block. If you use a guard clause, then you can also unindent the logic for handling a valid case where there is something to remove.

if item not in self.inventory:
     return False

self.inventory.remove(item)
return item

Another approach we could take is to try to remove the item directly, and handle the ValueError that occurs if it's not there, and return False to handle it (try/except)

[yangashley]

Looks good.

This method is a great candidate for using list comprehension if you want to refactor it since our conditional statement we check is pretty simple.

General syntax for list comp:

result_list = [element for element in source_list if some_condition(element)]

Which here would look like:

items_by_category = [item for item in self.inventory if item.category == category]

You can also forgo saving the list to the variable items_by_category and do something like:

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

[yangashley]

Nice guard clause

[yangashley]

You can remove this check here on line 57 & 58. If there is nothing in category_items, then when you loop nothing will happen.

However, you set best_item to = category_items[0] on line 60.

So if category_items is empty, you would incorrectly return an item. However, you can do best_item = None on line 60 instead and so if category_items is empty you achieve the requirement of returning None.

[yangashley]

Because of how swap_items is implemented, checking if my_best_item and their_best_item are valid here isn't necessary. See line 31 in vendor.py - you have a check in there that makes sure the args passed into swap_items() are valid.

Also swap_items() returns True if swapping happened and False if no swapping happened. Therefore, you can leverage the return value from swap_items() and refactor this method so it looks like this:

def swap_best_by_category(self, other, my_priority, their_priority):
        my_best_item = self.get_best_by_category(their_priority)
        their_best_item = other.get_best_by_category(my_priority)
        return self.swap_items(other, my_best_item, their_best_item)

[yangashley]

👍

[yangashley]

You can combine lines 84 and 85 if you want:
assert len(tai.inventory) == 3 and len(jesse.inventory) == 3

You can use list destructing and write lines 86-91 lines like this too - a pythonic way of doing it:

assert [item_d, item_e, item_c] == jesse.inventory
assert [item_a, item_b, item_f] == tai.inventory

https://medium.com/@umaramanat66/destructuring-list-in-python-like-javascript-f7d4c0968538

[yangashley]

👍