Ruby-Sarah Kim #63
Conversation
…ching movie adding to watched
…nal test case for an edge case
…from favorite list of movies
mmcknett-ada
left a comment
mmcknett-ada
left a comment
There was a problem hiding this comment.
Great job! Green 🟢
Overall comments:
- Great job implementing the new test cases
- Think about whether you could reduce the time complexity of some of the deeply-nested loops you have.
- Your code is well-formatted and easy to read!
| def get_available_recs(user_data): | ||
| recommended_movies = [] | ||
| user_not_watched = get_friends_unique_watched(user_data) | ||
| for movie_dict in user_not_watched: | ||
| if movie_dict["host] in user_data["subscriptions]: | ||
| recommended_movies.append(movie_dict) | ||
| return recommended_movies |
There was a problem hiding this comment.
Looks like you got some code in your markdown. 😄
| assert movie in updated_data["watchlist"] | ||
| assert FANTASY_2 in updated_data["watchlist"] | ||
|
|
||
| @pytest.mark.skip() |
There was a problem hiding this comment.
[opinion] I appreciate that you removed these instead of just commenting them out.
| assert average == pytest.approx(0.0) | ||
|
|
||
| @pytest.mark.skip() | ||
| def test_calculates_watched_average_rating(): |
There was a problem hiding this comment.
Nice, I love seeing tests added when the existing tests don't cover the behavior. 😄
|
|
||
| # Assert | ||
| assert len(friends_unique_movies) == 3 | ||
| assert friends_unique_movies == [FANTASY_4, HORROR_1, INTRIGUE_3] |
There was a problem hiding this comment.
[nit] I'd probably assert each individually, similar to test_friends_unique_movies. Asserting equality with the list forces a specific order as well.
| movie["title"] = title | ||
| movie["genre"] = genre | ||
| movie["rating"] = rating | ||
| return movie |
There was a problem hiding this comment.
Rather than creating an empty dictionary, then assigning the keys, we can use dictionary literal syntax to create the dictionary all at once.
movie_dict = { 'title': title, 'genre': genre, 'rating': rating }
return movie_dictIt could even be done all on one line
return { 'title': title, 'genre': genre, 'rating': rating }| # ------------- WAVE 3 -------------------- | ||
| # ----------------------------------------- | ||
| def get_most_watched_genre(user_data): | ||
| favorite_genre ={} |
There was a problem hiding this comment.
[naming] I'd pick something like genre_counts for this. It holds multiple genres, not just a single favorite genre.
| for user_movies in user_data["watched"]: | ||
| user_watched_movie.append(user_movies) | ||
| # Check to see if the user movie title is within the whole set of friend's watched list | ||
| for movie_info in user_watched_movie: |
There was a problem hiding this comment.
Since user_watched_movie is just a copy of user_data["watched"] and you never modify the watched list, you can just iterate over it directly:
for movie_info in user_data["watched"]:| def get_friends_unique_watched(user_data): | ||
| movie_friend_watched = [] | ||
| only_friends_watched = [] | ||
| for watched_list in user_data["friends"]: |
There was a problem hiding this comment.
[naming] I prefer calling this dictionary a friend instead of a watched_list since then you get the watched list by accessing friend["watched"].
for friend in user_data["friends"]:
for movie in friend["watched"]:| for movie_info in movie_friend_watched: | ||
| if movie_info not in user_data["watched"]: |
There was a problem hiding this comment.
What's the big-O time complexity of this loop? (Consider what in has to do in the if statement.)
| #generated a list of all the movies friends watched | ||
| for watched_list in user_data["friends"]: | ||
| for movie in watched_list["watched"]: | ||
| if movie not in friend_watched: |
There was a problem hiding this comment.
[perf] Another big-O question here -- what does this if statement do to the big-O time complexity of the overall loop?
2 participants
No description provided.