Ada C22 Phoenix Class Madina Dzhetegenova #20
Conversation
There was a problem hiding this comment.
Good work! Please review my comments, and let me know if you have any questions. Please feel free to ask me questions directly in Slack, or after you've had a chance to review this together, we could set up a meeting to follow up on anything you have questions about.
| @@ -158,13 +158,13 @@ def test_moves_movie_from_watchlist_to_empty_watched(): | |||
| # Assert | |||
| assert len(updated_data["watchlist"]) == 0 | |||
| assert len(updated_data["watched"]) == 1 | |||
|
|
|||
| raise Exception("Test needs to be completed.") | |||
| assert updated_data["watched"][0]["title"] == MOVIE_TITLE_1 | |||
There was a problem hiding this comment.
While it's probably the case that if the title is correct, the rest of the data is as well, we should check all three pieces of data to be sure. We can either do this by checking them individually, or we could make a movie dictionary instance to compare.
| @@ -182,13 +182,14 @@ def test_moves_movie_from_watchlist_to_watched(): | |||
| # Assert | |||
| assert len(updated_data["watchlist"]) == 1 | |||
| assert len(updated_data["watched"]) == 2 | |||
|
|
|||
| raise Exception("Test needs to be completed.") | |||
| assert HORROR_1 in updated_data["watched"] | |||
There was a problem hiding this comment.
I like the use of an in check here, since that won't care where in the watched list the movie ends up.
It would probably also be good to ensure that the movie left in the watchlist is the movie we expect, and that the other movie that started in the watched is still there.
| assert INTRIGUE_3 in friends_unique_movies | ||
| assert FANTASY_4 in friends_unique_movies | ||
| assert HORROR_1 in friends_unique_movies |
There was a problem hiding this comment.
👍 Great way to check tht the three expected movies are there without worrying about at what position each ended up.
| # Act | ||
| recommendations = get_new_rec_by_genre(sonyas_data) | ||
|
|
||
| # Assert | ||
| assert len(recommendations) == 0 |
There was a problem hiding this comment.
👍 This test needed both the act and the assert.
| @@ -1,23 +1,170 @@ | |||
| import copy | |||
There was a problem hiding this comment.
The copy module provides deepcopy, but we don't need that in this project. In particular, in the function deepcopy is being used, we're only reading the user_data structure. Check out the feedback in get_friends_unique_watched for more.
| def get_rec_from_favorites(user_data): | ||
| my_data = user_data["favorites"].copy() | ||
|
|
||
| friends_data = user_data["friends"] | ||
| for watched in friends_data: | ||
| their_watched = watched["watched"] | ||
| for their_movie in their_watched: | ||
| for i in range(0, len(my_data)): | ||
| if their_movie["title"] != my_data[i]["title"]: | ||
| del my_data[i] | ||
| break | ||
| return my_data |
There was a problem hiding this comment.
👀 This version of the function is replaced by the later one starting on line 159. Python lets us redefine a function (the "name" of a function is really just a variable which can be overwritten), so whatever implementation comes last will be the one that remains available.
There was a problem hiding this comment.
This approach is very similar to the logic in get_unique_watched except that it's starting from the user favorites rather than the user watched movies. We could remove the logic duplication either by:
- Starting with the unique user watched movies and filtering them to those that are also favorites, or
- Writing a helper function that filters an arbitrary movie list against the friends data, and use that function for both
get_unique_watchedandget_rec_from_favorites, passing in either the user watched list or favorite list as needed.
| for movie in user_data["favorites"]: | ||
| my_data[movie["title"]] = movie |
There was a problem hiding this comment.
👍 This chunk of the code makes an "index" of the user's favorite movies by title.
| friends_data = user_data["friends"] | ||
| for watched in friends_data: | ||
| their_watched = watched["watched"] | ||
| for their_movie in their_watched: | ||
| title = their_movie["title"] | ||
| my_data.pop(title, None) |
There was a problem hiding this comment.
👍 This code iterates through all the friends watched movies, eliminating the ones from the favorite index by title. Popping by key is a constant time operation, so this is pretty efficient! When we're done, only favorite movies not watched by any friends will remain in the index.
| for their_movie in their_watched: | ||
| title = their_movie["title"] | ||
| my_data.pop(title, None) | ||
| return my_data.values() |
There was a problem hiding this comment.
👍 This returns the movie records for the favorite movies not watched by the friends. Note that this does not return a list. It returns a dictionary value iterator. The prompt asks that we return a list, so this result should be passed to the list function to make a list of the values.
return list(my_data.values())There was a problem hiding this comment.
A lot of the logic in this function is dealing with excluding movies the friends have watched. What if we started with a list of movies only the user has watched (we have a helper from a previous wave for this!), and filtered that list to only those in the favorites list?
| if their_movie["host"] not in subscriptions: | ||
| continue |
There was a problem hiding this comment.
I typically use a continue in a loop more when there is a main chunk of code that can be run without fruther indentation once the continue case has been handled. So like here, I'd tend to write this logic as we have here only if the main logic that follows (the appending steps) could happen without further indentation.
No description provided.