Zoisite - Gwen/Raissa #36
Conversation
There was a problem hiding this comment.
Nice job on Viewing Party, Gwen and Raissa!
Your project shows that you both are getting more fluent with working with nested data structures, iterating over them, and using conditional logic!
Nice job making frequent commits!
I noticed that your PR was opened against the AdaGold repository. If you look at the top of this PR Github says "raissaglauciewants to merge 16 commits into AdaGold:main from raissaglaucie:main". Just be sure when you open your next project PR that the "base repository" points to "Ada-C19/project-name".
| 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 |
| # ******************************************************************************************* | ||
| # ****** Add assertions here to test that the correct movie was added to "watched" ********** | ||
| # ******************************************************************************************* | ||
| assert updated_data["watched"][1] == movie_to_watch |
There was a problem hiding this comment.
We could also check that the movie we expect to be left in the watchlist list is still there, and that the other watched movie is properly in the watched list.
| assert FANTASY_4 in friends_unique_movies | ||
| assert HORROR_1 in friends_unique_movies | ||
| assert INTRIGUE_3 in friends_unique_movies | ||
| assert amandas_data != clean_wave_3_data() |
| #act | ||
| recommendations = get_new_rec_by_genre(sonyas_data) | ||
| #assert | ||
| assert len(recommendations) == 0 |
| if title and genre and rating: | ||
| movie_dict = {} | ||
| movie_dict['title'] = title | ||
| movie_dict['genre'] = genre | ||
| movie_dict['rating'] = rating | ||
|
|
||
| return movie_dict |
There was a problem hiding this comment.
If we've passed the guard clause, then we know that title/genre/rating have valid values and we do not need to check for this condition on line 6. Instead, would prefer for lines 7-12 to be outside the conditional and indented at the main level of the method.
A note about guard clauses - The Guard Clause pattern helps in these cases by “inverting” that way of thinking and rapidly returning false conditions and keeping the important code blocks (usually the longest) in the lowest indentation level possible. If title/genre/rating are valid, then we don't need to have the rest of logic indented in an else statement. We can unindent them and have them in the main body of the function.
You can read more about guard clauses here
You can also create your dictionary with literal syntax and return it without first creating the variable movie_dict:
if not title or not genre or not rating:
return None
return { "title": title, "genre": genre, "rating": rating }| continue | ||
| if movies_dict['title'] not in user_watched_list: | ||
| friends_unique_movies.append(movies_dict) | ||
| return friends_unique_movies |
There was a problem hiding this comment.
Nice job. How could you use sets to implement this method?
What if you created a set for friends' movies and a set for user's movies, but the movies in the sets could be represented with a string that is the title of the movie. Then you could find the difference between friend and user, then loop through the result of the difference to append the movies to friends_unique_movies
| # ----------------------------------------- | ||
| # ------------- WAVE 4 -------------------- | ||
| # ----------------------------------------- | ||
| def get_available_recs(user_data): | ||
| friends_movies = get_friends_unique_watched(user_data) |
There was a problem hiding this comment.
Nice job reusing a method you already wrote
| for movie_dict in friends_movies: | ||
| if movie_dict['host'] in subscriptions: |
There was a problem hiding this comment.
What's the time complexity of a for loop that has to execute the in operator on a list during every iteration?
We could improve the subscription lookup if we trade a little space and make a set of the subscriptions (which are strings, so can go in a set). Using in on a list is a linear operation, but on a set, it's constant.
subscription_lookup = set(user_data["subscriptions"])
for movie in friends_watched:
if movie["host"] in subscription_lookup:
recommended.append(movie)|
|
||
|
|
||
| def get_rec_from_favorites(user_data): | ||
| user_favorites = user_data['favorites'] #list of dictionaries |
There was a problem hiding this comment.
Instead of leaving a comment about what the variable is, we should try to name the variable in a descriptive way that enables to remember what returns from user_data['favorites'], maybe something like user_favorites_list
| for friends_dict in user_data['friends']: | ||
| movies_list = friends_dict['watched'] #list of dictionary movies | ||
| for movies in movies_list: | ||
| friends_watched.append(movies) |
There was a problem hiding this comment.
This code reads really similar to what you wrote in your helper function get_friends_movies, let's reuse it here!
No description provided.