tigers_Samiya_Ismail #150
tigers_Samiya_Ismail #150
Conversation
There was a problem hiding this comment.
Overall, well done, Samiya! The ideas are definitely there, and there are definitely some really sleek pockets of code, they just need to be cleaned up in places. The one major flaw here is the fact that you have not completed all the assertions we asked of you. As a result, I do have to give this code a yellow, which is passing, just with some revisions needed@
| @@ -118,13 +118,15 @@ def test_moves_movie_from_watchlist_to_empty_watched(): | |||
| # Assert | |||
| assert len(updated_data["watchlist"]) == 0 | |||
| assert len(updated_data["watched"]) == 1 | |||
| #assert updated_data["watched"]==MOVIE_TITLE_1 | |||
There was a problem hiding this comment.
The correct assert here would be:
assert updated_data["watched"][0]["title"] == MOVIE_TITLE_1
The way you currently have it set up checks the entire "watched" list against the movie title.
| @@ -118,13 +118,15 @@ def test_moves_movie_from_watchlist_to_empty_watched(): | |||
| # Assert | |||
| assert len(updated_data["watchlist"]) == 0 | |||
| assert len(updated_data["watched"]) == 1 | |||
| #assert updated_data["watched"]==MOVIE_TITLE_1 | |||
There was a problem hiding this comment.
With the correct assert working, you would also want to check the genre and rating as well! Hint: Simply change the "title" to "genre" and "rating"
| @@ -143,12 +145,12 @@ def test_moves_movie_from_watchlist_to_watched(): | |||
| assert len(updated_data["watchlist"]) == 1 | |||
| assert len(updated_data["watched"]) == 2 | |||
|
|
|||
| raise Exception("Test needs to be completed.") | |||
| #raise Exception("Test needs to be completed.") | |||
There was a problem hiding this comment.
Missing an assert statement! See if you can assert that the movie_to_watch is in the updated data!
| @@ -55,12 +55,12 @@ def test_friends_unique_movies_not_duplicated(): | |||
| # Assert | |||
| assert len(friends_unique_movies) == 3 | |||
|
|
|||
| raise Exception("Test needs to be completed.") | |||
| #raise Exception("Test needs to be completed.") | |||
There was a problem hiding this comment.
Once we have moved the movies over, we need to assert that INTRIGUE_3, HORROR_1 and FANTASY_4 are all in the friends_unique_movies dictionary.
| @@ -53,12 +53,12 @@ def test_new_genre_rec_from_empty_friends(): | |||
| ] | |||
| } | |||
|
|
|||
| raise Exception("Test needs to be completed.") | |||
| #raise Exception("Test needs to be completed.") | |||
There was a problem hiding this comment.
We're missing an act and assert statement here! Create the recommendations and then make sure the resulting list is empty!
| for movie in friends_watched: | ||
| if movie["host"] in user_data.get("subscriptions"): | ||
| friends_sub.append(movie) | ||
| return friends_sub |
There was a problem hiding this comment.
This function is pretty much perfect! Great use of your helper functions! The one small tweak I would make is to make variable names that are a little more descriptive of what they are!
|
|
||
| def get_new_rec_by_genre(user_data): | ||
| user_genre=get_most_watched_genre(user_data) | ||
| friends_movies=get_friends_unique_watched(user_data) |
| friends_movies=get_friends_unique_watched(user_data) | ||
| user_movie_rec=[] | ||
| if not user_data["watched"]: | ||
| return user_movie_rec |
There was a problem hiding this comment.
Because we have initialized user_movie_rec as an empty list, this if statement and return statement are redundant. We don't need to worry about whether or not user_data["watched"] is empty!
| if not user_data["watched"]: | ||
| return user_movie_rec | ||
| for movie in friends_movies: | ||
| if movie ["genre"]in user_genre: |
There was a problem hiding this comment.
user_genre is a string, so an equality operator would be better used here. (== instead of in)
| favorite_movie.append(movie) | ||
| for movie in user_movies: | ||
| if movie in user_movies: | ||
| friends_movie_list.append(movie) |
There was a problem hiding this comment.
The if statements for these two for loops are the exact same condition as what is in the for loop. As a result, everything will actually be added to our lists, so these loops aren't really doing anything. If you combine both loops into one with a condition from each, you'd be good!
hiii