Skip to content

Cheetahs - Emma#86

Open
Emmakizil wants to merge 7 commits intoAda-C18:masterfrom
Emmakizil:master
Open

Cheetahs - Emma#86
Emmakizil wants to merge 7 commits intoAda-C18:masterfrom
Emmakizil:master

Conversation

@Emmakizil
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@jbieniosek jbieniosek 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 Emma! The code is very clean and easy to read. Fantastic use of helper methods in the Vendor class! This project is green.

Comment thread swap_meet/clothing.py
Comment thread swap_meet/item.py
Comment on lines +9 to +21
def condition_description(self):
if self.condition == 0:
return "Very Poor"
elif self.condition == 1:
return "Poor"
elif self.condition == 2:
return "Fair"
elif self.condition == 3:
return "Good"
elif self.condition == 4:
return "Very Good"
elif self.condition == 5.0:
return "Exceptional"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Conditions can be a float, but this function will only work for whole numbers. If an item has a condition of 3.5, this function will return None. How could you adapt these conditionals to work for floats?

Comment thread swap_meet/vendor.py
Comment thread swap_meet/vendor.py
Comment thread swap_meet/vendor.py
Comment on lines +39 to +40
self.add(friend.inventory.pop(0))
friend.add(self.inventory.pop(0))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could you use swap_items to simplify the code here?

Comment thread swap_meet/vendor.py
Comment thread swap_meet/vendor.py
Comment thread tests/unit_tests/test_wave_01.py
Comment thread tests/unit_tests/test_wave_06.py
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