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.

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' #1434

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Empty file.
7 changes: 7 additions & 0 deletions app/cinema/bar.py
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}.")
19 changes: 19 additions & 0 deletions app/cinema/hall.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
from app.people.cinema_staff import Cleaner
from app.people.customer import Customer


class CinemaHall:
def __init__(self, number: int) -> None:
self.number = number
Comment on lines +6 to +7

Choose a reason for hiding this comment

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

It would be a good idea to add a docstring to the __init__ method to explain its purpose and the arguments it takes. This can be beneficial for other developers who might work on this code in the future.


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

Choose a reason for hiding this comment

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

The movie_session method could use a docstring to explain what it does, what arguments it takes, and what it returns. This will enhance readability and maintainability of the code.

print(f'"{movie_name}" started in hall number {self.number}.')
for customer in customers:
customer.watch_movie(movie_name)
print(f'"{movie_name}" ended.')
cleaner.clean_hall(self.number)
Comment on lines +15 to +19

Choose a reason for hiding this comment

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

Inside this method, you have a sequence of operations executed one after another. This is a sign that these operations could be encapsulated into separate methods, following the Single Responsibility Principle. For example, you could create a start_movie method, a watch_movie method, an end_movie method, and a clean_hall method. This would make the movie_session method more readable and easier to maintain.

29 changes: 25 additions & 4 deletions app/main.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,27 @@
# write your imports here
from app.cinema.bar import CinemaBar
from app.cinema.hall import CinemaHall
from app.people.customer import Customer
from app.people.cinema_staff import Cleaner


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,

Choose a reason for hiding this comment

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

The type hint for customers is list[dict], which is not very descriptive. It would be better to create a data class or a NamedTuple for a Customer and use it as a type hint. This way, you can ensure that all Customer objects have a uniform structure and it's clear what fields a Customer object should have.

cleaner: str,
movie: str
) -> None:
customer_instances = [
Customer(name=customer["name"], food=customer["food"])
for customer in customers
]
ch = CinemaHall(number=hall_number)
cleaner_instance = Cleaner(name=cleaner)

for customer in customer_instances:
CinemaBar.sell_product(customer=customer, product=customer.food)

Comment on lines +20 to +21

Choose a reason for hiding this comment

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

In this loop, you are selling a product to each customer. However, it's not clear what happens if the product is not available or if the customer doesn't have enough money. It's recommended to handle these cases.

ch.movie_session(
movie_name=movie,
customers=customer_instances,
cleaner=cleaner_instance
)
Empty file added app/people/__init__.py
Empty file.
6 changes: 6 additions & 0 deletions app/people/cinema_staff.py
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

Choose a reason for hiding this comment

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

Code Quality Issue: It is a good practice to add docstrings to your classes and methods. Docstrings provide a convenient way of associating documentation with Python modules, functions, classes, and methods.


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

Choose a reason for hiding this comment

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

Code Quality Issue: The clean_hall method currently only prints a message. If this method is intended to perform actual cleaning operations, it might be a good idea to include a TODO comment to indicate future implementation.

7 changes: 7 additions & 0 deletions app/people/customer.py
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

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