-
Notifications
You must be signed in to change notification settings - Fork 72
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
Rock- Briyana Haywood #65
base: master
Are you sure you want to change the base?
Conversation
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 job Briyana!! You hit all the learning goals and I feel like you have a solid understanding of lists and dictionaries. I made some recommendations on how to refactor and make your code more scalable.
Keep up the great work! Looking forward to looking at these reviews together!
new_movie = { | ||
"title": "", | ||
"genre": "", | ||
"rating": 0 | ||
} | ||
if title and genre and rating: | ||
new_movie["title"] = title | ||
new_movie["genre"] = genre | ||
new_movie["rating"] = rating | ||
return new_movie | ||
else: | ||
return None |
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.
You can combine this code to look like:
if title and genre and rating:
new_movie = {
"title": title,
"genre": genre,
"rating": rating
}
return new_movie
else:
return None
for key, value in user_data.items(): | ||
user_data[key].append(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.
So at this point in the project, there is only one key ["watched"]. While this works, it is best to make functions as reusable as you can.
This can be refactored to user_data["watched"].append(movie)
for key, value in user_data.items(): | ||
user_data[key].append(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.
See comment above, can be refactored in the same way
for movies in user_data["watchlist"]: | ||
if title == movies["title"]: | ||
user_data["watched"].append(movies) | ||
user_data["watchlist"].pop(0) |
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.
Using the index to remove an item works now, however, what happens if the movie is in a different index?
for movies in user_data["watched"]: | ||
genre_list.append(movies["genre"]) | ||
for genres in genre_list: | ||
if genres in word_counter: | ||
word_counter[genres] += 1 | ||
else: | ||
word_counter[genres] = 1 |
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 frequency map!
word_counter[genres] += 1 | ||
else: | ||
word_counter[genres] = 1 | ||
most_watched = sorted(word_counter, key = word_counter.get, reverse = True) |
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 the sorted function
friends_movie_list = [] | ||
for friends_watched_movies in user_data["friends"]: | ||
for watched, movies in friends_watched_movies.items(): | ||
friends_movie_list.append(movies) | ||
comprehensive_friend_list = (friends_movie_list[0] + friends_movie_list[1]) | ||
return comprehensive_friend_list |
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 is a great helper function, however, I am curious as to why you're adding specific movies by id to the comprehensive_friend_list.
def get_unique_watched(user_data): | ||
friends_list = get_friends_movie_list(user_data) | ||
my_movie_list = [] | ||
unique_movies = [] | ||
for my_watched_movies in user_data["watched"]: | ||
my_movie_list.append(my_watched_movies) | ||
for items in my_movie_list: | ||
if items not in friends_list: | ||
unique_movies.append(items) | ||
return unique_movies | ||
|
||
def get_friends_unique_watched(user_data): | ||
friends_list = get_friends_movie_list(user_data) | ||
my_movie_list = [] | ||
unique_movies = [] | ||
shared_movies = [] | ||
for my_watched_movies in user_data["watched"]: | ||
my_movie_list.append(my_watched_movies) | ||
for items in friends_list: | ||
if items not in my_movie_list: | ||
shared_movies.append(items) | ||
for movie_items in shared_movies: | ||
if movie_items not in unique_movies: | ||
unique_movies.append(movie_items) | ||
return unique_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.
Nice! Great use of helper functions
No description provided.