Conversation
beccaelenzil
left a comment
beccaelenzil
left a comment
There was a problem hiding this comment.
Great work on your first project. Your code is clear, readable, and you've met the learning goals. You've made good use of list comprehensions, and your use of helper function is particularly notable. I've left a few inline comments on ways you might consider refactoring. I look forward to discussing in our 1:1.
|
|
||
|
|
||
| def most_frequent(list_value): | ||
| count = Counter(list_value) |
There was a problem hiding this comment.
Good work researching this library. It would be a useful exercise to build a frequency map yourself: https://www.geeksforgeeks.org/counting-the-frequencies-in-a-list-using-dictionary-in-python/ .
| new_movie={ | ||
| "title": movie_title, | ||
| "genre": genre, | ||
| "rating": rating} | ||
| return new_movie |
There was a problem hiding this comment.
You can move this code outside the else. In general if indentation isn't necessary, we prefer less indentation
| ''' | ||
| helper functions | ||
| ''' | ||
| def search_value(user_data,movie,search_for): |
There was a problem hiding this comment.
This is a clever way to use a helper function. Consider using a docstring (https://www.python.org/dev/peps/pep-0257/) and/or giving the function a bit more meaningful name to enhance readability.
| create_user_watched_list(user_data, user_watched_list) | ||
| create_friends_watched_list(user_data,friends_watched_list) |
There was a problem hiding this comment.
These helper functions are super handy and clear. Nice work!
| create_user_watched_list(user_data, user_watched_list) | ||
| create_friends_watched_list(user_data,friends_watched_list) | ||
| unique_in_friends = set(friends_watched_list).difference(set(user_watched_list)) | ||
| result = [{"title":item} for item in unique_in_friends] |
There was a problem hiding this comment.
Nice use of a list comprehension to get the data in the shape it needs to be
| user_list = [item['title'] for item in user_data['watched']] | ||
| host_list = user_data['subscriptions'] | ||
| create_friends_list(user_data,friends_list) | ||
| for i in friends_list: |
There was a problem hiding this comment.
Variable names like i are ok for counter variables, but try to provide more meaningful variable names for other use cases. In this case you might consider naming it movie.
| elif (not user_list and i['host'] in host_list) and (i not in recommendations): | ||
| recommendations.append(i) |
There was a problem hiding this comment.
It seems to me that this second conditional test is not necessary. I believe the difference from the first test is it's checking an if user_list is empty, however if user_list is empty, i['title'] will not be in user list. Thus, it looks like your test case counts for this scenario.
| favorites = [item['title'] for item in user_data["favorites"]] | ||
| friends_list = [] | ||
| create_friends_watched_list(user_data,friends_list) | ||
| not_watched = set(favorites).difference(set(friends_list)) |
There was a problem hiding this comment.
Nice work trying out sets for this problem.
2 participants
I just watched the git lessons. I will work on my commit messages on the next project!