Solution #1427
Solution #1427
Conversation
There was a problem hiding this comment.
Great job on your code! I can see that you've put a lot of effort into it. However, there are a few areas that need improvement. The movie_session method could be better organized for readability and maintainability. Also, consider using list comprehension for efficiency and ensure to add docstrings to your functions. Keep up the good work! 😊👍
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| def movie_session( | ||
| self, | ||
| movie_name: str, | ||
| customers: list["Customer"], | ||
| cleaning_staff: "Cleaner" | ||
| ) -> None: | ||
| print(f'"{movie_name}" started in hall number {self.number}.') | ||
| for customer in customers: | ||
| customer.watch_movie(movie_name) | ||
| print(f'"{movie_name}" ended.') | ||
| cleaning_staff.clean_hall(self.number) |
There was a problem hiding this comment.
The movie_session method is doing too much. It's not only starting and ending the movie session but also managing the customers watching the movie and the cleaning of the hall. This violates the Single Responsibility Principle and makes the method harder to read and maintain. Consider breaking this method into smaller methods, each with a single responsibility. For example, you could have separate methods for starting the movie, ending the movie, managing the customers, and cleaning the hall.
| def cinema_visit( | ||
| customers: list, | ||
| hall_number: int, | ||
| cleaner: str, | ||
| movie: str | ||
| ) -> None: |
There was a problem hiding this comment.
The function cinema_visit could benefit from a docstring that describes its purpose, parameters, and return value. This is a good practice and it will make your code easier to understand for other developers.
| cleaner_obj = Cleaner(cleaner) | ||
| hall = CinemaHall(hall_number) | ||
| cinema_bar = CinemaBar() |
There was a problem hiding this comment.
The instantiation of Cleaner, CinemaHall, and CinemaBar objects inside the cinema_visit function might limit the reusability of this function. If you want to use this function with different instances of these classes, you would have to modify the function. A better approach might be to pass these objects as parameters to the function.
| for customer in customers: | ||
| customer_obj = Customer(customer["name"], customer["food"]) | ||
| cinema_bar.sell_product( | ||
| product=customer_obj.food, | ||
| customer=customer_obj | ||
| ) | ||
| customer_objs.append(customer_obj) |
There was a problem hiding this comment.
Here you are creating a Customer object for each customer in the customers list, and then appending it to the customer_objs list. This operation could be more efficiently done using a list comprehension. This would make your code more concise and more Pythonic.
| hall.movie_session( | ||
| movie_name=movie, | ||
| customers=customer_objs, | ||
| cleaning_staff=cleaner_obj | ||
| ) |
There was a problem hiding this comment.
The function call hall.movie_session is a bit hard to read due to the number of arguments and their formatting. It could be improved by using one line per argument. This would increase the readability of your code.
No description provided.