Skip to content

Ocelots - Ya-Juan Ruan#17

Open
ryjpink wants to merge 3 commits intoada-ac2:masterfrom
ryjpink:master
Open

Ocelots - Ya-Juan Ruan#17
ryjpink wants to merge 3 commits intoada-ac2:masterfrom
ryjpink:master

Conversation

@ryjpink
Copy link
Copy Markdown

@ryjpink ryjpink commented Nov 12, 2022

Solve viewing party assignment.

Comment thread viewing_party/party.py

def create_movie(title, genre, rating):
pass
if title and genre and rating:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Interesting use of python "Truthyness" to validate for "" or None

Comment thread viewing_party/party.py
def watch_movie(user_data, title):
movie_watchlist = user_data["watchlist"]
# TODO: Should we check if the movie has already been watched?
for index, movie in enumerate(movie_watchlist):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

interesting way to loop, using enumerate to get an index!

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In general, it's considered dangerous practice to modify the content of a data structure that your looping over. This can lead to bugs in more complex code. One alternative could be to find another way without using specific indices, for example, just keeping a copy of a list of movies in movie_watchlist and looping over those. That way, inside that loop, modifying movie_watchlist would be perfectly safe.

Comment thread viewing_party/party.py
most_watched_genre = None
max_amount = 0
for genre, times in genre_map.items():
# if there is a tie, keep the first
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great comment, making it clear that you thought out the tie case

Comment thread viewing_party/party.py
# -----------------------------------------

# return a dictionary, key is title, value is a dictionary of movie's information
def get_friends_watched_movies(friends):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good implementation. For this and similar functions, could you think about using sets to simplify some of this code?

Comment thread viewing_party/party.py
def get_friends_unique_watched(user_data):
friends = user_data["friends"]
watched_movies = user_data["watched"]
user_watched = set()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

was there a specific reason that a set was used here?

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.

2 participants