Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Solution #1296

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Solution #1296

wants to merge 2 commits into from

Conversation

oshumska
Copy link

No description provided.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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


Hello! 😊 It looks like you're almost there, but there are a few critical issues that need to be addressed before we can approve the pull request:

  1. Animal Class Initialization: The Animal class should initialize the appetite directly from the argument, regardless of the is_hungry state. Currently, it sets appetite to 0 if is_hungry is False, which doesn't align with the task requirements.

  2. Cat and Dog Class Initialization: Similarly, the Cat and Dog classes should initialize with a fixed appetite of 3 and 7, respectively, without checking the is_hungry state. The current approach incorrectly sets appetite to 0 if is_hungry is False.

  3. Feed Animals Function: The feed_animals function should sum the food points returned by the feed method, not directly from the appetite attribute. This ensures that only hungry animals contribute to the total food points.

These changes are crucial to ensure the functionality aligns with the task requirements. Once these are addressed, your code will be in great shape! Keep up the good work, and I'm sure you'll nail it! 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.

app/main.py Outdated Show resolved Hide resolved
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Review Summary

Great job on implementing the Animal class and its subclasses Cat and Dog, as well as the feed_animals function! Your code meets the task requirements and constraints, and you've demonstrated a solid understanding of object-oriented programming concepts. 🎉

However, there are a few areas for improvement:

  1. Type Hinting: In the feed_animals function, consider specifying the type of elements in the animals list using List[Animal] from the typing module. This will enhance type checking and make your code more robust.

  2. Appetite Management: Setting self.appetite to 0 after feeding might not be ideal if you need to retain the original appetite value for future use. Consider resetting self.appetite only if it's necessary for your application logic.

These suggestions are meant to help you refine your code and improve its functionality. Remember, addressing these issues will be beneficial for your learning journey. Keep up the great work, and continue to learn from your experiences! 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

Comment on lines +15 to +16
appetite = self.appetite
self.appetite = 0

Choose a reason for hiding this comment

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

Setting self.appetite to 0 after feeding might not be the best approach if you want to retain the original appetite value for future use. Consider resetting self.appetite only if necessary for your application logic.

print("The slippers delivered!")


def feed_animals(animals: list) -> int:

Choose a reason for hiding this comment

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

The feed_animals function should specify the type of elements in the animals list for better type checking. Consider using List[Animal] from the typing module for the parameter type hint.

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