-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
test solution sent via git bash #1407
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 task requirements! 🎉 The core functionality is working well, and the class structure is correctly set up. Just a few minor adjustments are needed for the type hints in your functions to ensure they work with all relevant subclasses. Keep up the good work and happy coding! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
app/main.py
Outdated
from app.players.dwarves.dwarf_warrior import DwarfWarrior | ||
|
||
|
||
def calculate_team_total_rating(players: List[object]) -> int: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The calculate_team_total_rating
function should accept a list of Player
objects, not object
. Update the type hint to List[Player]
to reflect this requirement.
app/main.py
Outdated
return total_rating | ||
|
||
|
||
def elves_concert(elves: List[ElfRanger]) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The elves_concert
function should accept a list of Elf
objects, not just ElfRanger
. Update the type hint to List[Elf]
to ensure it works with all elf subclasses.
app/main.py
Outdated
elf.play_elf_song() | ||
|
||
|
||
def feast_of_the_dwarves(dwarves: List[DwarfWarrior]) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The feast_of_the_dwarves
function should accept a list of Dwarf
objects, not just DwarfWarrior
. Update the type hint to List[Dwarf]
to ensure it works with all dwarf subclasses.
There was a problem hiding this 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 project! There are a few critical issues that need to be addressed to fully meet the task requirements. Specifically, the type hints in your functions need to be updated to ensure they accept the correct class types. This includes adjusting the calculate_team_total_rating
function to accept a list of Player
objects, and ensuring that elves_concert
and feast_of_the_dwarves
accept lists of Elf
and Dwarf
objects, respectively. Additionally, please verify that the Elf
class constructor properly accepts a musical_instrument
parameter. Once these changes are made, your code will be in great shape! Keep up the great work! 🌟
✨ 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 app.players.dwarves.dwarf_warrior import DwarfWarrior | ||
|
||
|
||
def calculate_team_total_rating(players: List[object]) -> int: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The calculate_team_total_rating
function should take a list of Player
objects, not object
. Update the type hint to List[Player]
to reflect this requirement.
app/main.py
Outdated
return total_rating | ||
|
||
|
||
def elves_concert(elves: List[ElfRanger]) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The elves_concert
function should accept a list of Elf
objects, not just ElfRanger
. Update the type hint to List[Elf]
to align with the task requirements.
app/main.py
Outdated
elf.play_elf_song() | ||
|
||
|
||
def feast_of_the_dwarves(dwarves: List[DwarfWarrior]) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The feast_of_the_dwarves
function should accept a list of Dwarf
objects, not just DwarfWarrior
. Update the type hint to List[Dwarf]
to align with the task requirements.
def __init__(self, nickname: str, | ||
musical_instrument: str, | ||
favourite_spell: str) -> None: | ||
super().__init__(nickname, musical_instrument) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that the Elf
class constructor accepts musical_instrument
as a parameter. If not, adjust the super().__init__
call accordingly.
No description provided.