C22/ Natalie Sen & Luqi Xie/ Viewing-Party#9
C22/ Natalie Sen & Luqi Xie/ Viewing-Party#9natsen1004 wants to merge 18 commits intoAda-C22:mainfrom
Conversation
apradoada
left a comment
apradoada
left a comment
There was a problem hiding this comment.
Overall, this looks great, y'all! It's getting a GREEN!
I've left just a few things for y'all to take a look at, but nothing that I'm too worried about! All of my suggestions are mostly just so y'all can apply certain concepts to your code moving forward! You should be very proud of what you did here! You are welcome to make changes if you'd like, but it isn't required!
| # ******************************************************************************************* | ||
| # ****** Add assertions here to test that the correct movie was added to "watched" ********** | ||
| # ******************************************************************************************* | ||
| assert MOVIE_TITLE_1 == updated_data["watched"][0]["title"] |
There was a problem hiding this comment.
Great test! Just a couple of things here! Typically, we are checking to see if our new data is equal to a static variable. All this means is that our output should be on the left and the static variable should be on the right! Swap those for a more standard test.
Also, just to make sure our tests are super comprehensive, it might be better to assert that the entire movie is there as opposed to just the title. This may look something like this:
assert updated_data["watched"][0] == {
"title": MOVIE_TITLE_1,
"genre": GENRE_1,
"rating": RATING_1
}
| # ******************************************************************************************* | ||
| # ****** Add assertions here to test that the correct movie was added to "watched" ********** | ||
| # ******************************************************************************************* | ||
| assert HORROR_1 in updated_data["watched"] |
There was a problem hiding this comment.
This looks great! You could also check to se if the correct index is the movie you're looking for!
| # ************************************************************************************************** | ||
| 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.
These look good! One other thing to think about is what this specific test is looking for. It's named test_friends_unique_movies_not_duplicated. This means we're checking to make sure there aren't any duplicates. The Arrange portion of this test ensures that there are two friends who have the same movie. Is there a way we could more robustly test for no duplicates?
| # ****** Complete the Act and Assert Portions of these tests ********** | ||
| # ********************************************************************* | ||
| recommendations = get_new_rec_by_genre(sonyas_data) | ||
| assert len(recommendations) == 0 |
There was a problem hiding this comment.
These are great! I love the way you implemented the Act portion of the test!
| if title and genre and rating: | ||
| return { | ||
| "title": title, | ||
| "genre": genre, | ||
| "rating": rating | ||
| } | ||
| else: | ||
| return None |
There was a problem hiding this comment.
This is good, but I have one small tweak to suggest! When we look at functions like this, it is important to look at it in terms of a guard clause and the normal flow of the function. If this function runs as expected, a movie gets created. If either the title, genre or rating are missing, we simply return none.
With this in mind, the guard clause is checking for a missing title, genre or rating, and the movie creation acts as the normal flow of the function. Typically, we would want the guard clause to be a stand alone if statement (no else) and we don't need to put everything else in the guard clause. This may look something like this:
def create_movie(title, genre, rating):
if not title or not genre or not rating:
return None
return {
"title": title,
"genre": genre,
"rating": rating
}
This is a small change, but this sort of format (guard clause, then regular flow of the function) is a more standard way of approaching it!
| def get_available_recs(user_data): | ||
|
|
||
| recommended_movies = [] | ||
| unique_movies = get_friends_unique_watched(user_data) |
There was a problem hiding this comment.
Great job using a previous wave's functions!
|
|
||
| for unique_movie in unique_movies: | ||
| for subscription in user_data["subscriptions"]: | ||
| if subscription == unique_movie["host"]: |
There was a problem hiding this comment.
This works, but I think it's always a good idea to let python do the heavy lifting when it comes to the loops. If we just checked to see if the unique movie's host was in subscriptions, we could avoid having to explicitly create a nested loop!
| return new_rec_by_genre | ||
|
|
||
| for movie in friends_unique_watched_list: | ||
| if movie["genre"] in most_watched_genre: |
There was a problem hiding this comment.
most_watched_genre should come in as a string, so it would be a better idea to just check and see if both values are equal to each other!
| def get_new_rec_by_genre(user_data): | ||
|
|
||
| most_watched_genre = get_most_watched_genre(user_data) | ||
| friends_unique_watched_list = get_friends_unique_watched(user_data) |
| fav_movies_list = user_data["favorites"] | ||
| recommended_movies = [] | ||
|
|
||
| if not user_data["favorites"]: |
There was a problem hiding this comment.
If you are going to create a variable that holds this information (fav_movies_list), then make sure you're using it consistently! If you decide not to use it, that works too as long as you are being consistent across the program. I think earlier you hold it in a list and then use it consistently in the function. Here, we just have a bit of a mismatch!
3 participants
No description provided.