-
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
Scissors - Laurel O #67
base: master
Are you sure you want to change the base?
Conversation
list_com = [v for i in user_data_user_data_key for k,v in i.items() \ | ||
if v == i[value]] |
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.
-
Variable names can greatly improve code readability.
-
When using list comprehension, simpler is often more readable:
list_com = [i[value] for i in user_data_user_data_key]
. For readability I recommend avoiding double loops in list comprehension (sometimes they are unavoidable, but they are much harder to read).
compare this function to (please ignore tab error):
def generate_list_from_key(movie_list, dict_key):
new_list = [movie[dict_key] for movie in movie_list]
return new_list
list_dic = user_data["watched"] | ||
avg_rating = 0.0 | ||
if len(list_dic) > 0: | ||
rating_list = [v for i in list_dic for k,v in i.items() if k=="rating"] |
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.
rating_list = [movie["rating"] for movie in list_dic]
genres = list_com_fun(user_data["watched"], "genre") | ||
if genres == []: | ||
return None | ||
return max(genres) |
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 you solve this if you didn't have access to the max function?
friend1 = list_com_fun(user_data["friends"][0]["watched"] , "title") | ||
friend2 = list_com_fun(user_data["friends"][1]["watched"] ,"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 function will pass the tests because they only have 2 friends, how would this function need to change to be generalized to a friend list of any size?
movie = {"title":v} | ||
movie_copy = movie.copy() | ||
answer.append(movie_copy) | ||
return answer |
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.
The tests for this function use a simplified version of the movie dictionary. However, our original definition of a movie was that a movie has a title, rating and genre. How could you create a list of the original movie dictionaries using the unique
list?
Note: movie.copy() is not needed here.
|
||
def get_available_recs(user_data): | ||
user_sub = user_data["subscriptions"] | ||
friend_sub = [i for i in user_data["friends"][0]["watched"] and \ |
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 function has the same limitation as get_unique_watched and get_friends_unique_watched. How could you account for an unknown number of friends?
return rec_host | ||
|
||
def get_new_rec_by_genre(user_data): | ||
fav_genre = get_most_watched_genre(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 use of a helper function!
Great work on this project! Some things to work on in the next project:
|
No description provided.