C22 Phoenix - Dana Delgado and Jen Nguyen#8
C22 Phoenix - Dana Delgado and Jen Nguyen#8ItsJenOClock wants to merge 21 commits intoAda-C22:mainfrom
Conversation
apradoada
left a comment
apradoada
left a comment
There was a problem hiding this comment.
🟢 GREEN! 🟢
Hey y'all, you should be super proud of the work you did here! This program looks great and has been well optimized and uses some strong concepts. I made a few suggestions, but nothing that requires an update!
One thought I had was where do you go from here? My biggest suggestion there is to look at some slightly more advanced topics and see if you can incorporate those into your code. Specifically I was thinking about list comprehensions. They are not required but I think they could add to the flow of your code! Just something to think about!
Really great job, you two!
| assert MOVIE_TITLE_1 not in updated_data["watchlist"] | ||
| assert updated_data["watched"][0]["title"] == MOVIE_TITLE_1 | ||
| assert updated_data["watched"][0]["genre"] == GENRE_1 | ||
| assert updated_data["watched"][0]["rating"] == RATING_1 |
There was a problem hiding this comment.
These assertions look great! Super robust and cover all the things that might need to be covered!
| # ****** Add assertions here to test that the correct movie was added to "watched" ********** | ||
| # ******************************************************************************************* | ||
| assert movie_to_watch not in updated_data["watchlist"] | ||
| assert movie_to_watch in updated_data["watched"] |
| def test_no_movie_input(): | ||
| # Arrange | ||
| movie = None | ||
| user_data = { | ||
| "watched": [], | ||
| "watchlist": [] | ||
| } | ||
|
|
||
| # Act | ||
| updated_data = add_to_watched(user_data, movie) | ||
| updated_data = add_to_watchlist(user_data, movie) | ||
|
|
||
| # Assert | ||
| assert len(updated_data["watched"]) == 0 | ||
| assert len(updated_data["watchlist"]) == 0 No newline at end of file |
There was a problem hiding this comment.
I love this additional test! Always feel free to add tests that our test suite might cover! My one suggestion here is to work on the name of your test! test_no_movie_input is a little vague and it's a good idea to try and include both the input and the expected result in the test name. If we look at the previous test, test_does_nothing_if_movie_not_in_watchlist, that might be a good guide for what this test name could look like!
| assert FANTASY_4 in friends_unique_movies | ||
| assert HORROR_1 in friends_unique_movies | ||
| assert INTRIGUE_3 in friends_unique_movies |
There was a problem hiding this comment.
This looks good! One thing I like about what y'all did was you checked for every movie that could be included in the list! I think this was a smart idea as it confirms that there is no duplicate INTRIGUE_3. One fact that you can leverage in a test is that we know exactly how the function should act and exactly what the output should look like!
While checking to see if a particular movie is in the resulting output is great, if you don't check everything else like the length and every other movie that should be in there, it's easy for something to slip through the cracks. I wonder if there is a way to simply test and see if the friends_unique_movies looks like what we are expecting...?!
| recommendations = get_new_rec_by_genre(sonyas_data) | ||
|
|
||
| # Assert | ||
| assert len(recommendations) == 0 |
There was a problem hiding this comment.
The act and assert here look good! What other asserts could we check that would achieve a similar purpose?
| friends_unique_movies = [] | ||
| for friend in user_data["friends"]: | ||
| for movie in friend["watched"]: | ||
| if (movie["title"] not in user_unique_watched and | ||
| movie not in friends_unique_movies): | ||
| friends_unique_movies.append(movie) |
There was a problem hiding this comment.
I see where y'all are going here, but I wonder if there is a way to avoid a triply nested loop here (technically the not in friends_unique_movies ends up being a loop)?
There was a problem hiding this comment.
Would using a set for friends_unique_movies instead of a list avoid the triply nested loop, and then we could return list(friends_unique_movies)?
| def get_available_recs(user_data): | ||
| friends_unique_movies = get_friends_unique_watched(user_data) | ||
| recommended_movies = [] | ||
|
|
||
| for movie in friends_unique_movies: | ||
| if movie["host"] in user_data["subscriptions"]: | ||
| recommended_movies.append(movie) | ||
| return recommended_movies | ||
|
|
There was a problem hiding this comment.
This function looks good! Great use of the previously built functions!
| for movie in friends_unique_movies: | ||
| if movie["genre"] == most_watched_genre: | ||
| recommended_movies.append(movie) | ||
| return recommended_movies |
| return recommended_movies | ||
|
|
||
| def get_rec_from_favorites(user_data): | ||
| # We made the assumption that the favorites are in "user_unique_watched" |
There was a problem hiding this comment.
Always a good idea to state your assumptions for anyone who might read your code after you've written it!
| for movie in user_data["favorites"]: | ||
| if movie in user_unique_watched: | ||
| recommended_movies.append(movie) | ||
| return recommended_movies |
There was a problem hiding this comment.
Ending on a great note! Everything here looks really good!
3 participants
No description provided.