Skip to content

Cheetahs - Lynn Jansheski#93

Open
1lynnj wants to merge 20 commits intoAda-C18:masterfrom
1lynnj:master
Open

Cheetahs - Lynn Jansheski#93
1lynnj wants to merge 20 commits intoAda-C18:masterfrom
1lynnj:master

Conversation

@1lynnj
Copy link

@1lynnj 1lynnj commented Oct 7, 2022

No description provided.

1lynnj added 20 commits October 3, 2022 11:59
…ategory function, all other unit tests passed
…ded tests to wave6 tests file for get_newest function
…eview. Did nothing to how code works and was inadvertantly left in.
Copy link

@kendallatada kendallatada left a comment

Choose a reason for hiding this comment

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

Hi Lynn! Your project has been scored as green.

There are several methods in your Vendor class where a try-except block could be implemented to improve time complexity. Checking to see if an item is in a list before doing something with it, is an O(n) operation. So, see if it's possible to replace those if statements with a try-except block instead. :)

Nice job! You can find my comments in your code. Let me know if you have any questions! 😊

result = vendor.remove(item)

raise Exception("Complete this test according to comments below.")
assert result == "item to remove"

Choose a reason for hiding this comment

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

Be sure to read the instructions and test names carefully! the Vendor remove method is supposed to return False if the item is not found.

]

for i in range(len(items)):
items[i].condition_description

Choose a reason for hiding this comment

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

Pay attention to what each line of your code is doing and if it serves a purpose. What is this loop doing? condition_description() is a method, but this line does not call it since it's missing the ending parentheses. Even if it were calling the method, the output is not being saved anywhere to be used again.

I'd recommend doing a pass over your code after it's working to do some refactoring and clean up. :)

assert one_condition_description != five_condition_description


##########Adding tests to improve code coverage

Choose a reason for hiding this comment

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

Nice! Love that you're paying attention to code coverage! ✨

# their inventory after swap
assert item_c in jesse.inventory
assert item_d in jesse.inventory
assert item_e in jesse.inventory

Choose a reason for hiding this comment

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

Good asserts 👍🏾

# - That tai and jesse's inventories are the correct length
# - That all the correct items are in tai and jesse's inventories

#############Adding tests for newest items

Choose a reason for hiding this comment

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

Nice job writing your own tests for newest items! 🥳

pass
from swap_meet.item import Item

class Electronics(Item):

Choose a reason for hiding this comment

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

Great job with wave 5 and using inheritance! Nice work hardcoding the category too 🥳


def remove(self, item):
'''Remove item from inventory list.'''
if item in self.inventory:

Choose a reason for hiding this comment

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

This approach works, but consider how a try-except block could improve your time complexity. 🤔 💭 😌

if len(self.get_by_category(category)) == 0:
return None
else:
return max(self.get_by_category(category), key=lambda item: item.condition)

Choose a reason for hiding this comment

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

Oooo! Great usage of the max() function with a lambda function!! 🤩


def get_newest_item(self):
'''Get newest item from inventory.'''
if len(self.inventory) > 0:

Choose a reason for hiding this comment

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

Since a list with 1 or more values is truthy, the Pythonic way to write this if statement would be:

if self.inventory:

😊 🐍

else:
return False

def swap_by_newest(self, other):

Choose a reason for hiding this comment

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

Great job with the optional enhancements! 🎉

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