Solution #1603
Solution #1603
Conversation
There was a problem hiding this comment.
Thank you for your hard work on this project! However, there are a few critical issues that need to be addressed before we can approve the pull request. Specifically, the parameter naming and type mismatches in several parts of the code, such as in the CinemaBar class and the cinema_visit function, need to be corrected to meet the task requirements. Additionally, ensure that the movie_session method in the CinemaHall class uses the correct parameter name. Once these issues are resolved, your code will be in great shape! Keep up the great work, and don't hesitate to reach out if you need any help. 😊
✨ 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
|
|
||
| class CinemaBar: | ||
| @staticmethod | ||
| def sell_product(product: str, customers: Customer) -> None: |
There was a problem hiding this comment.
The parameter name customers should be customer to match the task description, which specifies that the sell_product method takes a single Customer instance as customer.
app/cinema/bar.py
Outdated
| class CinemaBar: | ||
| @staticmethod | ||
| def sell_product(product: str, customers: Customer) -> None: | ||
| print(f"Cinema bar sold {product} to {customers} ") |
There was a problem hiding this comment.
The placeholder in the formatted string should use customer.name instead of customers to correctly display the customer's name. Ensure the parameter name is corrected to customer as well.
app/main.py
Outdated
| from app.people.cinema_staff import Cleaner | ||
|
|
||
|
|
||
| def cinema_visit(customers: Customer, hall_number: CinemaHall, cleaner: Cleaner, movie: CinemaHall): |
There was a problem hiding this comment.
The parameter types in the function signature are incorrect. customers should be a list of dictionaries, hall_number should be an integer, and movie should be a string. Update the function signature to reflect these types.
| customer_instances = [Customer(name=c["name"], food=c["food"]) for c in customers] | ||
| cleaning_staff = Cleaner(name=cleaner) | ||
| for customer in customer_instances: | ||
| CinemaBar.sell_product(product=customer.food, customer = customer) |
There was a problem hiding this comment.
The sell_product method call has a parameter naming issue. The correct parameter name should be customer instead of customer to match the method definition in CinemaBar.
| cleaning_staff = Cleaner(name=cleaner) | ||
| for customer in customer_instances: | ||
| CinemaBar.sell_product(product=customer.food, customer = customer) | ||
| cinema_hall = CinemaHall(number=hall_number) |
There was a problem hiding this comment.
The hall_number parameter should be an integer, not a CinemaHall instance. Update the instantiation of CinemaHall to use the correct type.
| for customer in customer_instances: | ||
| CinemaBar.sell_product(product=customer.food, customer = customer) | ||
| cinema_hall = CinemaHall(number=hall_number) | ||
| cinema_hall.movie_session(movie_name=movie, customers=customer_instances, cleaning_staff=cleaning_staff) |
There was a problem hiding this comment.
The movie_name parameter in movie_session should be a string, not a CinemaHall instance. Ensure that the movie parameter is passed as a string.
There was a problem hiding this comment.
Great job on your submission! 🎉 While your implementation is mostly correct, there are a few areas that need attention to fully align with the task requirements:
-
Parameter Naming in
movie_sessionMethod: Ensure that the parameter name is consistent with the task description. It should bemovie_nameinstead ofmovies_name. -
Parameter Naming in
cinema_visitFunction: The parameter should becustomersto accurately represent that it is a list, as per the task description. -
CleanerClass Initialization: Thehall_numberparameter in the constructor is redundant and should be removed since theclean_hallmethod already takes ahall_numberparameter. -
Output Formatting in
CinemaBar: Modify theprintstatement to correctly display the customer's name by accessing thenameattribute of theCustomerinstance.
These issues are important for the functionality of your code, and addressing them will enhance your understanding and skills. Keep up the good work, and use this feedback to improve your code in future tasks. Remember, learning from these experiences is key to becoming a better developer! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| class CinemaBar: | ||
| @staticmethod | ||
| def sell_product(product: str, customer: Customer) -> None: | ||
| print(f"Cinema bar sold {product} to {customer} ") |
There was a problem hiding this comment.
The print statement currently outputs the Customer object directly, which may not display the customer's name as intended. You should modify the statement to access the name attribute of the Customer instance, like this: print(f"Cinema bar sold {product} to {customer.name}").
| from app.people.cinema_staff import Cleaner | ||
|
|
||
|
|
||
| def cinema_visit(customer: list, hall_number: int, cleaner: str, movie: str): |
There was a problem hiding this comment.
The parameter customer should be renamed to customers to accurately reflect that it is a list of customer dictionaries, as per the task description.
| for customer in customer_instances: | ||
| CinemaBar.sell_product(product=customer.food, customer = customer) | ||
| cinema_hall = CinemaHall(number=hall_number) | ||
| cinema_hall.movie_session(movie_name=movie, customers=customer_instances, cleaning_staff=cleaning_staff) |
There was a problem hiding this comment.
The parameter movie_name in the movie_session method call should be movies_name to match the parameter name defined in the CinemaHall class.
| @@ -0,0 +1,7 @@ | |||
| class Cleaner: | |||
| def __init__(self, name: str, hall_number: int) -> None: | |||
There was a problem hiding this comment.
The hall_number parameter in the constructor is redundant because the clean_hall method already accepts a hall_number parameter. Consider removing hall_number from the constructor to simplify the class design.
No description provided.