Skip to content

C22 - Phoenix - Elzara Turakulova#3

Open
ielzara wants to merge 9 commits intoAda-C22:mainfrom
ielzara:main
Open

C22 - Phoenix - Elzara Turakulova#3
ielzara wants to merge 9 commits intoAda-C22:mainfrom
ielzara:main

Conversation

@ielzara
Copy link

@ielzara ielzara commented Oct 3, 2024

No description provided.

@ielzara ielzara changed the title C22 - Elzara Turakulova C22 - Phoenix Elzara Turakulova Oct 3, 2024
@ielzara ielzara changed the title C22 - Phoenix Elzara Turakulova C22 - Phoenix - Elzara Turakulova Oct 3, 2024
Copy link

@apradoada apradoada left a comment

Choose a reason for hiding this comment

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

GREEN! Well done, Elzara! I am so sorry for the late feedback, but honestly, this project shows me that you have a really good grasp of python. You've used ternaries and list comprehensions in all the places you needed to and your optional enhancements and tests all look really great! Well done!


def __str__(self):
parent_str = super().__str__()
return f"{parent_str} It is made from {self.fabric} fabric."

Choose a reason for hiding this comment

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

I really like the way you've parsed out the parent_str here. Feel free to skip that step and just include the super().__str__() in the f String!


def __str__(self):
parent_str = super().__str__()
return f"{parent_str} It takes up a {self.width} by {self.length} sized space."

Choose a reason for hiding this comment

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

This function looks great! Good use of the default parameters!


def __str__(self):
parent_str = super().__str__()
return f"{parent_str} This is a {self.type} device."

Choose a reason for hiding this comment

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

Looks good!

class Item:
pass No newline at end of file
def __init__(self, id=None, condition=0, age=0):
self.id = uuid.uuid4().int if id is None else id

Choose a reason for hiding this comment

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

Great ternary here!

elif x < 5:
return "Almost new, if you squint hard enough."
else:
return "Mint condition! Did you steal this from a time machine?"

Choose a reason for hiding this comment

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

Your Item class looks really good too! I love the descriptions you have for your conditions! I also appreciate that you have given ranges for each condition description! this allows for a wider range of possibilities!

Comment on lines +53 to +56
best_item = None
for item in items:
if best_item is None or item.condition > best_item.condition:
best_item = item

Choose a reason for hiding this comment

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

A super small tweak, but if we can avoid having to check and see if best_item is None along with the more important condition of comparing the conditions, we should try that!

best_from_other = other_vendor.get_best_by_category(my_priority)
best_from_self = self.get_best_by_category(their_priority)

return self.swap_items(other_vendor, best_from_self, best_from_other)

Choose a reason for hiding this comment

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

As always, this looks good too!

Comment on lines +67 to +85
def find_newest_item(self, items):
if not items:
return None

newest_item = None
for item in items:
if newest_item is None or item.age < newest_item.age:
newest_item = item

return newest_item

def swap_by_newest(self, other_vendor):
my_new_item = self.find_newest_item(self.inventory)
their_new_item = self.find_newest_item(other_vendor.inventory)

if not my_new_item or not their_new_item:
return False

return self.swap_items(other_vendor, my_new_item, their_new_item) No newline at end of file

Choose a reason for hiding this comment

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

Thank you for adding in these optional enhancements! They look really good!

assert len(jesse.inventory) == 3
assert item_a in jesse.inventory
assert item_d in tai.inventory
assert item_b in tai.inventory No newline at end of file

Choose a reason for hiding this comment

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

Your optional test suite looks great!

Comment on lines +35 to +36
assert len(items) == 0
assert items == []

Choose a reason for hiding this comment

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

These two asserts are testing for a very similar outcome. I would argue that only one over the other is needed! Specifically, I would check for the empty list as that covers your other assert as well!

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