Sphinx - Leidy Martinez & Aleida Vieyra #14
Conversation
…last 3 tests for test_wave_01.py
…ction get_friends_unique_watched(user_data)
| movie= {} | ||
|
|
||
| if movie_title and genre and rating: | ||
|
|
||
| movie["title"] = movie_title | ||
| movie["genre"] = genre | ||
| movie["rating"] = rating | ||
| return movie | ||
|
|
||
| return None |
There was a problem hiding this comment.
One way to shorten this could be like the following:
def create_movie(movie_title, genre, rating):
if not movie_title or not genre or not rating:
return None
return {
"title": movie_title,
"genre": genre,
"rating": rating
}I would be okay with this since all we are doing is creating a dictionary representation of a movie and then return it.
| # Create a copy of user data to avoid modifying original | ||
| updated_user_data = { | ||
| "watched": user_data["watched"][:]} |
There was a problem hiding this comment.
I see that you took into consideration that user_data was a mutable data type and decided to make a copy of the structure to avoid modifications of the original. I love that you are thinking about things like and applying your learnings from class! In this case, its actually fine to modify the original user_data since they ask you too in the requirements. Since the updated structure is returned and that return value is what is tested all your tests pass. If we were to test against the original user_data it would not.
| # and add it to the watched list | ||
| for movie in updated_user_data["watchlist"]: | ||
| if movie["title"] == title: | ||
| updated_user_data["watchlist"].remove(movie) |
There was a problem hiding this comment.
So now that we have talked about Big O analysis, we know that .remove has a time complexity of O(n) since it needs to look through every element in a list to find the sought after value. Right now, this loop has an outer loop of O(n) and you when you find the movie to remove you have another O(n) operation. How could we circumvent using remove? (Hint: Think about list comprehension or pop method
Also in this implementation, we will only use remove once, but in other filtering problems we might have to do this for multiple elements so it doesn't hurt to get in the habit now.
| # Count and sum rating values | ||
| for movie in user_data["watched"]: | ||
| if movie["rating"]: | ||
| count += 1 |
There was a problem hiding this comment.
Do we need to keep count of the number of movies a user has watched or can we just use a built in function provided to us by python instead in this function? 👀
| for key, current_value in genre_frequencies.items(): | ||
| if current_value > max_value: | ||
| max_value = current_value | ||
| most_watched = key #retrieve the key with the highest value |
| if movie["genre"] in genre_frequencies: | ||
| genre_frequencies[movie["genre"]] += 1 | ||
| else: | ||
| genre_frequencies[movie["genre"]] = 1 |
There was a problem hiding this comment.
How could we use the .get method so that we could condense this to a single line? 👀
| for friend in user_data["friends"]: | ||
| for movie in friend['watched']: | ||
| friends_watched.append(movie) |
There was a problem hiding this comment.
Love the flattening of your data structures so you can simplify your loops! Also what if we just added the title of movies to friends_watched instead of the whole movie?
| # Get every movie from user and add it to a list | ||
| # only if is not in friends_wacthed list | ||
| for movie in user_data["watched"]: | ||
| if movie not in friends_watched: |
There was a problem hiding this comment.
Since friends_watched is a list, this line has a time complexity of O(n) (n being the number of movies in a list). What other structure could we use in order to circumvent the linear time complexity and get a constant one?
| # AT LEAST ONE OF THE FRIENDS HAS WATCHED, | ||
| # BUT THE USER HAS NOT | ||
|
|
||
| user_watched = [movie for movie in user_data["watched"]] |
There was a problem hiding this comment.
Omg, list comprehension! Love to see her! 😍
| if movie not in user_watched\ | ||
| and movie not in friends_unique_watched: |
| if movie["host"] in user_data["subscriptions"]: | ||
| rec_list.append(movie) | ||
|
|
||
| return rec_list |
| # Check the movies in friends that correspond to the same genre | ||
| # the user watch the most and add it to a list | ||
| for movie in friends_unique_movies: | ||
| if user_freq_genre in movie.values(): |
There was a problem hiding this comment.
Would it be more declarative to say:
if movie["genre"] == user_freq_genre This more directly communicates that you are only adding movies that match the genre of a user's favorite genre
| if movie in user_data["favorites"]: | ||
| rec_list.append(movie) | ||
|
|
||
| return rec_list |
| # ********************************************************************* | ||
| # ****** Complete the Act and Assert Portions of these tests ********** | ||
| assert len(genre_recommendations) == 0 |
There was a problem hiding this comment.
What if we did assert len(genre_recommendations) == []? This more readily declares that the genre_recommendations should be an empty list. Also for testing, if someone returned, say an empty string, this test would technically pass.
| # ************************************************************************************************* | ||
| # ****** Add assertions here to test that the correct movies are in friends_unique_movies ********** | ||
| assert friends_unique_movies == [FANTASY_4, HORROR_1, INTRIGUE_3] | ||
| assert amandas_data["watched"] not in friends_unique_movies |
There was a problem hiding this comment.
So, this assert statement here is checking if the entire list of watched movies is a single element inside of friends_unique_movies. How could we change this to check if the individual movies are inside of friends_unique_watched?
| # ******************************************************************************************* | ||
| # ****** Add assertions here to test that the correct movie was added to "watched" ********** | ||
| assert updated_data["watched"][0]["title"] == MOVIE_TITLE_1 |
| janes_data = { | ||
| "watchlist": [FANTASY_1], | ||
| "watched": [FANTASY_2, movie_to_watch] | ||
| } | ||
| assert updated_data == janes_data |
There was a problem hiding this comment.
Only suggestion I have here is to change name to something else like expected. Also see my note below about making copies of the originals.
No description provided.