Ocelots - Dalia Ali#10
Conversation
kelsey-steven-ada
left a comment
kelsey-steven-ada
left a comment
There was a problem hiding this comment.
🎉 Congrats on completing your first project at Ada! 🎉 I’ve added some suggestions & questions, let me know if there's anything I can clarify.
| # Assert | ||
| assert len(updated_data["watchlist"]) == 0 | ||
| assert len(updated_data["watched"]) == 1 | ||
| assert updated_data["watched"][0]["title"] == MOVIE_TITLE_1 |
There was a problem hiding this comment.
Nice assertion! In this case where the code is checking individual keys of a dictionary, I recommend including assertions for all of the relevant keys.
watched_movie = updated_data["watched"][0]
assert watched_movie["title”] == MOVIE_TITLE_1
assert watched_movie["rating"] == RATING_1
assert watched_movie["genre"] == GENRE_1| # Assert | ||
| assert len(updated_data["watchlist"]) == 1 | ||
| assert len(updated_data["watched"]) == 2 | ||
| assert updated_data["watched"][1]["title"] == "It Came from the Stack Trace" |
There was a problem hiding this comment.
The README isn't explicit about what order we should add movies to the list watched. If we want our tests to be independent from a specific implementation that adds movies to a particular location in the watched list, we could use the in keyword to check if the variable movie_to_watch is in the watched list instead of looking at a specific index. By checking for the whole dictionary, there's a side benefit that we don't need to check each key independently:
assert movie_to_watch in updated_data["watched"]
# Another option:
assert HORROR_1 in updated_data["watched"]| assert INTRIGUE_3 in friends_unique_movies | ||
| assert HORROR_1 in friends_unique_movies | ||
| assert FANTASY_4 in friends_unique_movies |
There was a problem hiding this comment.
Great checks for all the relevant data!
| recommendations = get_new_rec_by_genre(sonyas_data) | ||
|
|
||
| #Assert | ||
| assert len(recommendations) == 0 |
| def create_movie(title, genre, rating): | ||
| pass | ||
| def create_movie(movie_title, genre, rating): | ||
| movies = {} |
There was a problem hiding this comment.
I suggest moving the movies declaration under the invalid data check so that we don't allocate space for movies unless we know it will be used.
I would also suggest a singular name like movie for this variable. A plural name in programming generally implies that the variable holds a collection of several items. Since this variable holds the data for a single movie, a singular name would better represent what it holds.
| # ----------------------------------------- | ||
| # ------------- WAVE 3 -------------------- | ||
| # ----------------------------------------- | ||
| def get_collective_friends_movies(user_data): |
There was a problem hiding this comment.
Really nice use of a helper function!
| return unique_movies | ||
|
|
||
|
|
||
| def get_friends_unique_watched(user_data): |
There was a problem hiding this comment.
Nice implementations for this wave! 😄
| # ------------- WAVE 4 -------------------- | ||
| # ----------------------------------------- | ||
| def get_available_recs(user_data): | ||
| friends_unique_movies = get_friends_unique_watched(user_data) |
| for movie in friends_unique_movies: | ||
| if movie["host"] in user_data["subscriptions"]: | ||
| available_recommendations.append(movie) |
There was a problem hiding this comment.
Nice algorithm! Another way we could approach filtering the unique_watched list is with a list comprehension:
# list comprehension
result = [movie for movie in friends_unique_movies if movie["host"] in user_data["subscriptions"]]This line is a bit long, in practice we would split a statement like this across lines, use some extra variables, or shorten some naming to keep under the PEP8 guide of 79 characters max per line:
# list comprehension
result = [movie for movie in friends_unique_movies
if movie["host"] in user_data["subscriptions"]]| friends_movies_list = get_collective_friends_movies(user_data) | ||
| recommendations = [] | ||
|
|
||
| for movie in user_data["favorites"]: | ||
| if movie not in friends_movies_list: | ||
| recommendations.append(movie) | ||
|
|
||
| return recommendations |
There was a problem hiding this comment.
I like this approach and that it uses your previous helper function! Another approach could use the result of get_unique_watched:
unique_movies = get_unique_watched(user_data)
recommendations = []
for movie in unique_movies:
if movie in user_data['favorites']:
recommendations.append(movie)
return recommendations2 participants
completed viewing party assignment