ElvesAndDwarves #1361
ElvesAndDwarves #1361
Conversation
There was a problem hiding this comment.
Thank you for your hard work on this project! While your code is mostly on the right track, there are a few critical issues that need to be addressed before we can approve the pull request:
-
Type Hinting: The
calculate_team_total_ratingfunction should useList[Player]for theteamparameter to align with the task requirements. This ensures that the function is flexible and can handle anyPlayertype. -
Return Type Annotations: The
get_ratingmethod in theDwarfBlacksmithclass should have a return type ofint, as it returnsself.skill_level, which is expected to be an integer. -
Parameter Type Hinting: The
skill_levelparameter in theDwarfBlacksmithclass should be typed asintinstead ofNoneto accurately reflect its data type. -
Protected Attributes: Ensure that attributes like
skill_level,hummer_level,bow_level, andfavourite_spellare prefixed with an underscore to indicate they are protected, as per the task requirements.
These changes are important to ensure the functionality and adherence to the task requirements. Once these are addressed, your code will be in great shape! Keep up the great work, and don't hesitate 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.
app/main.py
Outdated
| from typing import List | ||
|
|
||
|
|
||
| def calculate_team_total_rating(team: List[Elf | Dwarf]) -> int: |
There was a problem hiding this comment.
The type hint for the team parameter should be List[Player] instead of List[Elf | Dwarf] to match the task description, which specifies that the function should take a list of Players.
| self, | ||
| nickname: str, | ||
| favourite_dish: str, | ||
| skill_level: None |
There was a problem hiding this comment.
The skill_level parameter should have a type hint that reflects its actual data type, such as int, instead of None.
| f"{self.nickname} with skill of the " | ||
| f"{self.skill_level} level") | ||
|
|
||
| def get_rating(self) -> None: |
There was a problem hiding this comment.
The get_rating method should have a return type hint of int instead of None, as it returns self.skill_level, which is expected to be an integer.
| musical_instrument: str, | ||
| favourite_spell: str | ||
| ) -> None: | ||
| super().__init__(nickname, musical_instrument) |
There was a problem hiding this comment.
Ensure that the Elf class constructor accepts nickname and musical_instrument as parameters. If the Elf class constructor has different parameters, adjust the super().__init__() call accordingly.
| print(f"" | ||
| f"{self.nickname} is playing a song on the " | ||
| f"{self.musical_instrument}") |
There was a problem hiding this comment.
| print(f"" | |
| f"{self.nickname} is playing a song on the " | |
| f"{self.musical_instrument}") | |
| print( | |
| f"{self.nickname} is playing a song on the " | |
| f"{self.musical_instrument}" | |
| ) |
No description provided.