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

Task solution #3330

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

Conversation

MateuszCieplak
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.


Thank you for your hard work on this implementation! 😊 While your code mostly meets the task requirements, there are a couple of critical issues that need to be addressed before we can approve the pull request:

  1. Use of filter Method: In the Carnivore class's bite method, you should replace the use of indexOf and splice with the filter method to remove animals with health less than or equal to 0 from the Animal.alive array. This change will improve performance and align with the task requirements.

  2. Type Checking in bite Method: Ensure that the bite method checks if the animal is an instance of Herbivore and is not hidden. You can use instanceof Herbivore to verify the type.

Once these issues are addressed, your code will be in great shape! Keep up the good work, and feel free to reach out if you have any questions or need further assistance. 🚀

✨ 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.

}

class Carnivore extends Animal {
// write your code here
bite(animal) {
if (animal.hidden === false) {

Choose a reason for hiding this comment

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

The bite method should check if the animal is an instance of Herbivore and not hidden. You can use instanceof Herbivore to ensure the correct type.

Comment on lines 32 to 33
if (animal.health <= 0 && Animal.alive.includes(animal)) {
Animal.alive.splice(Animal.alive.indexOf(animal), 1);

Choose a reason for hiding this comment

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

Instead of manually checking and splicing the Animal.alive array, consider using the filter method to remove animals with health less than or equal to 0. This approach is more concise and aligns with the task requirements.

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.

Great job on implementing the Animal, Herbivore, and Carnivore classes! 🎉 Your code aligns well with the task requirements, particularly in the Carnivore class where the bite method correctly filters out animals with health less than or equal to 0 and checks for Herbivore instances. The hide method in the Herbivore class is also implemented correctly.

While the code is approved and ready for use, remember to review the comments for any potential improvements. Addressing these will help you enhance your coding skills and avoid similar issues in the future. Keep up the good work and continue learning from each task! 🚀

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

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