Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Solution #1431

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Solution #1431

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file added app/cinema/__init__.py
View file
Open in desktop
Empty file.
7 changes: 7 additions & 0 deletions app/cinema/bar.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
from app.people.customer import Customer


class CinemaBar:
@staticmethod
def sell_product(customer: Customer, product: str) -> None:
print(f"Cinema bar sold {product} to {customer.name}.")
18 changes: 18 additions & 0 deletions app/cinema/hall.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
from app.people.customer import Customer
from app.people.cinema_staff import Cleaner


class CinemaHall:
def __init__(self, number: int) -> None:
self.number = number

def movie_session(
self, movie_name: str,
customers: list[Customer],
cleaning_staff: Cleaner
) -> None:
Comment on lines +9 to +13
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the movie_session method, you are passing customers as a list of Customer objects and cleaning_staff as a Cleaner object. This tightly couples your CinemaHall class to the Customer and Cleaner classes, making your code less flexible and harder to maintain. A better approach would be to pass these as generic objects and use duck typing. This way, you can pass any object that has a watch_movie method for customers and a clean_hall method for cleaning_staff. This is a code quality issue.

print(f"\"{movie_name}\" started in hall number {self.number}.")
for customer in customers:
customer.watch_movie(movie_name)
print(f"\"{movie_name}\" ended.")
Comment on lines +14 to +17
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are using print statements to log the start and end of the movie. While this is fine for a simple script, in a larger application you would want to use a logging framework. This would give you more control over your log messages, such as the ability to write them to a file or send them to a remote server, and to control their format and level of detail. This is a code quality recommendation.

cleaning_staff.clean_hall(self.number)
24 changes: 20 additions & 4 deletions app/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,22 @@
# write your imports here
from app.people.customer import Customer
from app.people.cinema_staff import Cleaner
from app.cinema.bar import CinemaBar
from app.cinema.hall import CinemaHall


def cinema_visit(customers: list, hall_number: int, cleaner: str, movie: str):
# write you code here
pass
def cinema_visit(
customers: list[dict], hall_number: int, cleaner: str, movie: str
) -> None:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function cinema_visit is doing a lot of things, from creating customer objects to selling products and starting a movie session. Consider breaking down this function into smaller functions, each with a single responsibility. This would make the code easier to understand, test, and maintain.

customer_lst = [
Customer(
name=customer["name"], food=customer["food"]
) for customer in customers
]
Comment on lines +11 to +13
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you're creating Customer objects based on the input list of dictionaries. It might be better to handle this elsewhere, perhaps in a separate factory function or in the Customer class itself. This way, the cinema_visit function would not need to know the internal structure of the customer data, making it more flexible and easier to change in the future.

cinema_hall = CinemaHall(number=hall_number)
cinema_bar = CinemaBar()
cleaner_ = Cleaner(name=cleaner)
for customer in customer_lst:
cinema_bar.sell_product(customer=customer, product=customer.food)
cinema_hall.movie_session(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are accessing customer.food directly here. It would be better to use a getter method to retrieve this information, to ensure that you're always getting the most up-to-date value and to encapsulate the internal data of the Customer class.

movie_name=movie, customers=customer_lst, cleaning_staff=cleaner_
)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The movie_session method seems to be doing a lot of things, including managing the customers and the cleaning staff. Consider breaking this down into smaller methods, each with a single responsibility. This would make the code easier to understand, test, and maintain.

Empty file added app/people/__init__.py
View file
Open in desktop
Empty file.
6 changes: 6 additions & 0 deletions app/people/cinema_staff.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class Cleaner:
def __init__(self, name: str) -> None:
self.name = name
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a good practice, it's recommended to use docstrings to provide a brief description about what the class or method does. This will enhance readability and understanding of your code.


def clean_hall(self, hall_number: int) -> None:
print(f"Cleaner {self.name} is cleaning hall number {hall_number}.")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid using print statements for non-debugging purposes. If you want to log some information, consider using Python's logging module. It provides a flexible framework for emitting log messages from Python programs.

7 changes: 7 additions & 0 deletions app/people/customer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class Customer:
def __init__(self, name: str, food: str) -> None:
self.name = name
self.food = food
Comment on lines +2 to +4
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be a good idea to add some input validation for the name and food parameters. For instance, you could check if they are non-empty strings. This would prevent potential issues later on when these values are used.


def watch_movie(self, movie: str) -> None:
print(f"{self.name} is watching \"{movie}\".")
Loading