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 #1430

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Solution #1430

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f59fc1e
Solution
mitichd Oct 11, 2024
9e383f5
Solution
mitichd Oct 11, 2024
fe9f42e
Solution
mitichd Oct 11, 2024
210abf9
Solution
mitichd Oct 11, 2024
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.
6 changes: 6 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,6 @@
class CinemaBar:


@staticmethod
def sell_product(product: str, customer) -> str:
Copy link

Choose a reason for hiding this comment

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

This function has a return type hint of str, but does not return anything. This can lead to confusion for other developers who use this function, expecting it to return a string. If there is nothing to return, you should remove the return type hint.

print(f"Cinema bar sold {product} to {customer.name}.")
Copy link

Choose a reason for hiding this comment

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

This function seems to be incomplete. It only prints a message but does not do anything else. Consider adding functionality to actually sell the product, for example, by decrementing the product from the bar's stock and adding it to the customer's purchases.

Copy link

Choose a reason for hiding this comment

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

You are calling customer.name assuming customer is an object, but according to your function signature, you are expecting a string. Make sure to pass the correct arguments to your function. Also, instead of printing the string, it would be better to return it, so you can use the result elsewhere if needed.

Copy link

Choose a reason for hiding this comment

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

The sell_product method currently only prints a message. It doesn't perform any action related to selling a product. Consider implementing the logic to handle the selling process, like checking product availability, deducting the product amount from the stock, checking if the customer has enough money, etc.

17 changes: 17 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,17 @@
from app.people.cinema_staff import Cleaner

class CinemaHall:


def __init__(self, number: int):
Copy link

Choose a reason for hiding this comment

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

The __init__ method only initializes the number attribute of the CinemaHall class. Consider adding more attributes to this class if they are necessary for its functionality. For example, you could add a movies attribute to keep track of the movies shown in this cinema hall.

self.number = number
Copy link

Choose a reason for hiding this comment

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

It's a good practice to validate input data. In this case, you should check if the number is a positive integer.


def movie_session(self, movie_name, customers: list, cleaning_staff) -> str:
Copy link

Choose a reason for hiding this comment

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

The type of movie_name and cleaning_staff parameters are not specified. It's a good practice to always specify the type of parameters in the method signature. This makes the code more readable and prevents potential bugs caused by unexpected data types.

# clin_name = Cleaner(cleaning_staff)
print(f"\"{movie_name}\" started in hall number {self.number}.")
for customer in customers:
customer.watch_movie(movie_name)
Copy link

Choose a reason for hiding this comment

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

It's assumed that customers is a list of Customer objects, but there's no validation of this. It's a good practice to check the type of elements in the list.

print(f"\"{movie_name}\" ended.")
cleaning_staff.clean_hall(self.number)
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 is doing too many things - it's starting the movie, making customers watch the movie, and then cleaning the hall. Consider splitting this method into several smaller methods, each with a single responsibility. This will make your code more modular and easier to maintain.

Copy link

Choose a reason for hiding this comment

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

Here, cleaning_staff is a string, but it's used as an object with a clean_hall method. It seems like a mistake.



27 changes: 23 additions & 4 deletions app/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,25 @@
# 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,
hall_number: int,
cleaner: str,
movie: str
Copy link

Choose a reason for hiding this comment

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

The cleaner parameter is expected to be a string. It would be more clear if the parameter was named cleaner_name.

) -> 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 lacks a docstring. It's important to provide a clear description of what your function does, what parameters it expects, and what it returns (if anything). This is crucial for other developers who might use your function, and it's also a good practice for you to clarify your own thinking and design. Here's an example of how you might write it:

"""
Simulates a visit to the cinema.

Parameters:

  • customers (list): A list of dictionaries, where each dictionary represents a customer and contains 'name' and 'food' keys.
  • hall_number (int): The number of the hall where the movie will be shown.
  • cleaner (str): The name of the cleaner.
  • movie (str): The name of the movie to be shown.
    """
This is just a suggestion, you should adjust it according to the actual functionality of your function.

name_and_food = [
Customer(customer["name"], customer["food"])
for customer in customers
]
cinema_bar = CinemaBar()
hall = CinemaHall(hall_number)
name_cleaner = Cleaner(cleaner)

Copy link

Choose a reason for hiding this comment

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

The creation of the Cleaner object could be moved inside the CinemaHall class. This way, the cinema_visit function would not need to know about the Cleaner class, and the CinemaHall class would be responsible for managing its own cleaner.

for customer in name_and_food:
cinema_bar.sell_product(customer.food, customer)

Comment on lines +32 to +33
Copy link

Choose a reason for hiding this comment

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

This loop could potentially raise an exception if the customer.food is not available in the CinemaBar. It would be better to handle such cases gracefully, for example by using a try-except block.

hall.movie_session(movie, name_and_food, name_cleaner)
Empty file added app/people/__init__.py
View file
Open in desktop
Empty file.
8 changes: 8 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,8 @@
class Cleaner:


def __init__(self, name: str):
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.

It's a good practice to use type annotations for instance variables, not only for function arguments. This will make your code more readable and will help other developers understand what type of data should be stored in the variable. You can add it like this: self.name: str = name.


def clean_hall(self, hall_number: int) -> str:
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.

The clean_hall method currently prints a message to the console. Consider returning the message instead of printing it. This would make your code more testable and reusable, as you can handle the returned message in whatever way you want outside of this method. For example, you may want to log this message to a file, display it in a GUI, etc. If you return the message, you can decide on the handling of the message outside of this method.

Copy link

Choose a reason for hiding this comment

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

Instead of using the print function to return the result, you should use the return statement. The print function is generally used for debugging or for providing information to the user, not for returning the result of a function. If you want to return a string, you should do something like this: return f'Cleaner {self.name} is cleaning hall number {hall_number}.'.

9 changes: 9 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,9 @@
class Customer:


def __init__(self, name: str, food: str):
Copy link

Choose a reason for hiding this comment

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

It's a good practice to include a docstring at the beginning of the class to describe its purpose. This makes your code more readable and maintainable.

self.name = name
self.food = food
Copy link

Choose a reason for hiding this comment

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

Docstrings are also recommended for methods to explain what they do. In this case, you should explain what the init method is doing and what the parameters 'name' and 'food' are.


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

Choose a reason for hiding this comment

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

The 'watch_movie' method currently doesn't return anything, but the function signature suggests it should return a string. If you want this method to return a string, you should use the 'return' statement instead of 'print'. If you intended to only display the message, you should remove '-> str' from the function signature.

Loading