-
Notifications
You must be signed in to change notification settings - Fork 81
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
Sapphire - Angie Contreras, Lyuda Kim #10
base: main
Are you sure you want to change the base?
Conversation
…passing all but Ada-C19#9, Celina's implementation (commented out) passing all but Ada-C19#9 and Ada-C19#10
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work Angie and Lyuda! You hit all the learning goals for this project and all tests are passing. You have earned a well-deserved 🟢 grade on this project ✨
I added comments, compliments, and hints on ways to refactor your code.
Keep up the great work! ✨
def create_movie(title, genre, rating): | ||
pass | ||
# Input validation | ||
if not title or not genre or not rating: | ||
return None | ||
|
||
movie = { | ||
"title": title, | ||
"genre": genre, | ||
"rating": rating | ||
} | ||
# Return the dictionary | ||
return movie |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Great use of a guard clause to exit the function early. Optionally, we could return the dictionary literal itself to save 1 line (a really minor nitpick).
def create_movie(title, genre, rating):
# Input validation
if not title or not genre or not rating:
return None
return {
"title": title,
"genre": genre,
"rating": rating
}
def add_to_watched(user_data, movie): | ||
if not movie: | ||
return user_data | ||
|
||
user_data["watched"].append(movie) | ||
|
||
return user_data | ||
|
||
def add_to_watchlist(user_data, movie): | ||
if not movie: | ||
return user_data | ||
|
||
user_data["watchlist"].append(movie) | ||
|
||
return user_data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Great work checking if the movie was valid first before adding to the watched/watchlist lists!
def watch_movie(user_data, title): | ||
|
||
for i in range(len(user_data['watchlist'])): | ||
title_from_dict = user_data['watchlist'][i]['title'] | ||
|
||
# If title is in watchlist, remove that movie from watchlist | ||
if title_from_dict == title: | ||
dict_from_list = user_data['watchlist'][i] | ||
user_data["watchlist"].remove(dict_from_list) | ||
# Add that movie to "watched" | ||
user_data["watched"].append(dict_from_list) | ||
|
||
return user_data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Nice work!
One thing to consider is that a ranged
loop is really useful when we need to swap items in a list and/or utilize the indices in a list. Because this feature primarily focuses on the values of the list ( the movie dictionaries), we can iterate through the list directly instead of indices (typically what a ranged loop like this is used for). Iterating over the items directly omits an index variable(which we aren't really utilizing anywhere within this function, we're only checking the value of the movie's title compared to the title that was passed to the function).
This suggestion would look like:
def watch_movie(user_data, title):
for movie in user_data['watchlist']:
# If title is in watchlist, remove that movie from watchlist
if movie["title"] == title:
user_data["watchlist"].remove(movie)
# Add that movie to "watched"
user_data["watched"].append(movie)
break
return user_data
I also have added a break
to prevent unnecessary iterations (For example, if the matching movie is found in the first iteration, we can break out of the loop early instead of having to iterate through the rest of the list.)
def get_watched_avg_rating(user_data): | ||
|
||
if user_data["watched"] == []: | ||
return 0.0 | ||
|
||
total_rating = 0 | ||
|
||
for i in range(len(user_data["watched"])): | ||
rating_from_dict = user_data['watched'][i]['rating'] | ||
total_rating += rating_from_dict | ||
num_of_movies = len(user_data["watched"]) | ||
return total_rating / num_of_movies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
def get_most_watched_genre(user_data): | ||
genres_most_watched = {} | ||
|
||
# Iterate through "watched" list | ||
for i in range(len(user_data['watched'])): | ||
genre = user_data['watched'][i]['genre'] | ||
if genre in genres_most_watched: | ||
genres_most_watched[genre] += 1 | ||
else: | ||
genres_most_watched[genre] = 1 | ||
|
||
highest_occurence = 0 | ||
most_watched = None | ||
|
||
# Iterate through out genres_most_watched dict | ||
for genre, num in genres_most_watched.items(): | ||
if num > highest_occurence: | ||
highest_occurence = num | ||
most_watched = genre | ||
|
||
return most_watched |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Nice use of a frequency map!
for i in range(len(user_watched)): | ||
user_watched_titles.append(user_watched[i]["title"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 This would be great to add to a helper function!
for dict in friends_list: | ||
for value in dict.values(): | ||
for movie in value: | ||
friends_title = movie["title"] | ||
|
||
for user_dict in user_watched: | ||
if friends_title not in user_watched_titles and movie["host"] in user_data["subscriptions"] and movie not in movie_recs: | ||
movie_recs.append(movie) | ||
|
||
return movie_recs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A quadruple loop! I highly recommend trying to see if we can reduce the number of nested loops. Tools like VS Code Debugger can be really helpful in visualizing the shape of the data we're working with.
def get_new_rec_by_genre(user_data): | ||
# New dict will hold the most watched genre | ||
user_watched_list = user_data["watched"] | ||
most_watched_genre = {} | ||
for movie in user_watched_list: | ||
value_genre = movie["genre"] | ||
if value_genre in most_watched_genre: | ||
most_watched_genre[value_genre] += 1 | ||
else: | ||
most_watched_genre[value_genre] = 1 | ||
|
||
# Max_watched_genre hold the value of the most watched genre by user | ||
max_watched_genre = "" | ||
max_watched = 0 | ||
for genre, num in most_watched_genre.items(): | ||
if num > max_watched: | ||
max_watched = num | ||
max_watched_genre = genre | ||
|
||
# Create a dict that'll hold the titles of user's watched movies | ||
user_watched_movies = {} | ||
for movie in user_watched_list: | ||
key = movie["title"] | ||
user_watched_movies[key] = 1 | ||
|
||
# List of recommended movies by most watched genre and user hasn't watched it. | ||
rec_movies_by_gender = {} | ||
friends_list = user_data["friends"] | ||
for dict in friends_list: | ||
for value in dict.values(): | ||
for movie in value: | ||
watched_by_friend = movie["title"] | ||
genre_movie_friend = movie["genre"] | ||
# La agrego si yo no la he visto y si es del genero que mas veo | ||
if watched_by_friend in rec_movies_by_gender: | ||
continue | ||
if watched_by_friend not in user_watched_movies and genre_movie_friend == max_watched_genre: | ||
rec_movies_by_gender[watched_by_friend] = movie | ||
|
||
return list(rec_movies_by_gender.values()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would this function change if we used get_most_watched_genre
as a helper function?
def get_rec_from_favorites(user_data): | ||
|
||
# List that holds the titles of the favorite movies. | ||
favorites_list = user_data["favorites"] | ||
friends_titles = [] | ||
|
||
# Access the list of movies that friends' have watched | ||
friends_watched_list = user_data["friends"] | ||
|
||
# Compare every item in the favorites list against the movies in friends_watched_list | ||
# Nested loops to access every title in friends_watched_list | ||
result_favs = [] | ||
for dict in friends_watched_list: | ||
for value in dict.values(): | ||
for movie_watched in value: | ||
title = movie_watched["title"] | ||
|
||
if title not in friends_titles: | ||
friends_titles.append(title) | ||
|
||
# Loop through Favorites list to compare titles | ||
for movie in favorites_list: | ||
favorite_title = movie["title"] | ||
# Check if the movie is in the user's favorites and None of its friends have watched | ||
if favorite_title not in friends_titles and movie not in result_favs: | ||
result_favs.append(movie) | ||
|
||
return result_favs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! How might this function change if we used get_friends_unique_watched
and get_most_watched_genre
as helper functions?
assert updated_data == {'watchlist': [{'title': 'The Lord of the Functions: The Fellowship of the Function', 'genre': 'Fantasy', 'rating': 4.8}], 'watched': [{'title': 'The Lord of the Functions: The Two Parameters', 'genre': 'Fantasy', 'rating': 4.0}, {'title': 'It Came from the Stack Trace', 'genre': 'Horror', 'rating': 3.5}]} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
No description provided.