Solution #1412
Solution #1412
Conversation
There was a problem hiding this comment.
Your code is well-structured and meets the task requirements effectively! 🎉 The class hierarchy and methods are implemented correctly, and you've done a great job managing the list of alive animals. While there are a couple of minor naming suggestions to improve clarity—like renaming print_alive to get_alive and adjusting the __str__ method for better alignment with its typical use—these do not impact the functionality. Keep up the great work, and consider these suggestions for future improvements! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
app/main.py
Outdated
| @classmethod | ||
| def __str__(cls) -> str: |
There was a problem hiding this comment.
The __str__ method is typically used to provide a string representation of an instance, not a class. Consider renaming this method to something more appropriate, like get_alive_repr, to avoid confusion.
app/main.py
Outdated
| def print_alive(cls) -> list: | ||
| return cls.alive |
There was a problem hiding this comment.
The method print_alive returns a list of alive animals, but the name suggests it prints something. Consider renaming it to get_alive to better reflect its functionality.
app/main.py
Outdated
| def __init__(self, | ||
| name: str, | ||
| health: int = 100 | ||
| ) -> None: |
There was a problem hiding this comment.
Fix code style, if you specify a parameter on a line with (, then the last parameter must have a ) !
2 options:
def init(self,
name: str,
health: int = 100) -> None:
def init(
self,
name: str,
health: int = 100
) -> None:
There was a problem hiding this comment.
Read please checklist (point 5) before pushing your solution.
app/main.py
Outdated
|
|
||
| class Carnivore(Animal): | ||
| @staticmethod | ||
| def bite(other: "Animal") -> None: |
There was a problem hiding this comment.
If you do not have any problems importing this class, you do not need to use “Animal”, just Animal
app/main.py
Outdated
| other.health -= 50 | ||
| if other.health <= 0: | ||
| other.health = 0 | ||
| Animal.remove_dead() |
There was a problem hiding this comment.
Using this method, you remove all animals with less than 0 health from the list at once, and I don't think this is right.
It would be more logical to remove a specific instance of an animal that was killed from the list, for example:
Animal.alive.remove(other)
or super().alive.remove(other)
app/main.py
Outdated
|
|
||
| class Carnivore(Animal): | ||
| @staticmethod | ||
| def bite(other: "Animal") -> None: |
There was a problem hiding this comment.
other is not a good variable naming. In my opinion, better would be something like "target".
app/main.py
Outdated
| if isinstance(target, Herbivore) and not target.hidden: | ||
| target.health -= 50 | ||
| if target.health <= 0: | ||
| target.health = 0 |
app/main.py
Outdated
| def __init__(self, | ||
| name: str, | ||
| health: int = 100) -> None: |
There was a problem hiding this comment.
One attribute per line, when you have a lot of attributes. At checklist you have a good example of styling
def long_function_name(
var_one,
var_two,
var_three,
var_four
) -> None:
app/main.py
Outdated
| @classmethod | ||
| def remove_dead(cls) -> None: | ||
| cls.alive = [animal for animal in cls.alive if animal.health > 0] | ||
|
|
||
| @classmethod | ||
| def get_alive_repr(cls) -> str: | ||
| return str([repr(animal) for animal in cls.alive]) | ||
|
|
||
| @classmethod | ||
| def get_alive(cls) -> list: | ||
| return cls.alive |
There was a problem hiding this comment.
these methods isn't needed due to task requirements
No description provided.