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

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

Solution #1266

wants to merge 5 commits into from

Conversation

Atikiho
Copy link

@Atikiho Atikiho commented Jul 23, 2024

No description provided.

cleaning_staff: cinema_staff.Cleaner) -> None:

print(f'"{movie_name}" started in hall number {self.number}.')
for someone in customers:

Choose a reason for hiding this comment

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

Bad variable name. Use the singular when iterating. For example: for car in cars

Comment on lines 17 to 18
one = customer.Customer(someone.name, "")
one.watch_movie(movie_name)

Choose a reason for hiding this comment

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

Why create another one from an already created customer? This is already an instance of the Customer class, so you should use hist methods right away.

Comment on lines 2 to 3
import app.people.customer as customer
import app.people.cinema_staff as cinema_staff

Choose a reason for hiding this comment

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

It is better to import a whole class and use it without as.

app/main.py Outdated
Comment on lines 15 to 16
cinema_hall = CinemaHall(hall_number)
cinema_hall.movie_session(movie, all_customers, Cleaner(cleaner))

Choose a reason for hiding this comment

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

You are not declaring a variable with Cleaner instance, but at the same time you are doing this for cinema_hall. We can use this instance without a variable

Comment on lines 10 to 13
def movie_session(self,
movie_name: str,
customers: list,
cleaning_staff: app.people.cinema_staff.Cleaner) -> None:

Choose a reason for hiding this comment

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

Pls, start all arguments from a new line if you have many of them(Check all funcs in your code), for example:

def movie_session(
    self,
    movie_name: str
) -> None:

Copy link
Contributor

@Y-Havryliv Y-Havryliv left a comment

Choose a reason for hiding this comment

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

gj!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants