-
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
'Solution' #1454
base: master
Are you sure you want to change the base?
'Solution' #1454
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.
Thank you for your hard work on this project! While your code is mostly aligned with the task requirements, there are a few critical issues that need to be addressed before we can approve the pull request:
-
Incorrect Return Type Annotations: Several methods have the return type annotation
-> print
, which is not valid. These methods should not specify a return type since they only print messages and do not return values. Please remove these incorrect annotations. -
Type Annotation Issues:
- The
customer
parameter in theCinemaBar
class should be annotated as aCustomer
instance, not a callable. - The
cleaning_staff
parameter in theCinemaHall
class should be annotated as aCleaner
instance, not a callable. - The
cleaner
parameter in thecinema_visit
function should be annotated as aCleaner
instance, not a string.
- The
These changes are necessary to ensure the code meets the core requirements and functions correctly. Once these issues are resolved, your pull request will be ready for approval. Keep up the great work, and don't hesitate to reach out if you 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/cinema/bar.py
Outdated
@@ -0,0 +1,4 @@ | |||
class CinemaBar: | |||
@staticmethod | |||
def sell_product(customer: callable, product: str) -> print: |
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 return type annotation -> print
is incorrect. The method should not specify a return type since it only prints a message and does not return a value. Remove the return type annotation.
app/cinema/bar.py
Outdated
@@ -0,0 +1,4 @@ | |||
class CinemaBar: | |||
@staticmethod | |||
def sell_product(customer: callable, product: str) -> print: |
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 customer
parameter is incorrectly annotated as callable
. According to the task description, it should be a Customer
instance. Update the type annotation to reflect this.
app/cinema/hall.py
Outdated
self.number = number | ||
|
||
def movie_session(self, movie_name: str, customers: list, | ||
cleaning_staff: callable) -> 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 cleaning_staff
parameter is incorrectly annotated as callable
. According to the task description, it should be a Cleaner
instance. Update the type annotation to reflect this.
app/main.py
Outdated
# write you code here | ||
pass | ||
def cinema_visit(customers: list, hall_number: int, | ||
cleaner: str, movie: str) -> 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 cleaner
parameter is incorrectly annotated as str
. According to the task description, it should be a Cleaner
instance. Update the type annotation to reflect this.
app/people/cinema_staff.py
Outdated
def __init__(self, name: str) -> None: | ||
self.name = name | ||
|
||
def clean_hall(self, hall_number: int) -> print: |
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 return type annotation -> print
is incorrect. The method should not specify a return type since it only prints a message and does not return a value. Remove the return type annotation.
app/people/customer.py
Outdated
self.name = name | ||
self.food = food | ||
|
||
def watch_movie(self, movie: str) -> print: |
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 return type annotation -> print
is incorrect. The method should not specify a return type since it only prints a message and does not return a value. Remove the return type annotation.
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! 🎉 Your code meets the specified requirements and constraints, and the overall structure is well-organized. Here are some key points from the review:
-
Correct Implementations: The
CinemaBar
,CinemaHall
,Customer
, andCleaner
classes are all implemented correctly according to the task requirements. The methods within these classes perform the expected actions and are well-defined. -
Main Module: The
cinema_visit
function inmain.py
orchestrates the cinema visit process correctly, using absolute imports as required. -
Issue to Address: There is a minor issue with the instantiation of the
Cleaner
class in thecinema_visit
function. Ensure that theCleaner
class is designed to be instantiated with a single argument as currently implemented. If not, please adjust the instantiation to match the constructor requirements.
Your code is approved to help you move forward with your learning journey. Please review the comments to improve your code further. Addressing the instantiation issue will enhance the functionality and robustness of your solution. Keep up the great work, and continue to learn from these experiences! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
for customer in customer_info: | ||
CinemaBar.sell_product(customer=customer, product=customer.food) | ||
hall = CinemaHall(hall_number) | ||
cleaner_name = Cleaner(cleaner) |
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 instantiation of Cleaner
with Cleaner(cleaner)
assumes that Cleaner
takes a single argument in its constructor. Verify that the Cleaner
class is designed to be instantiated this way. If Cleaner
requires different arguments or initialization, adjust this line accordingly.
No description provided.