Charday Neal and Elzara Turakulova #3
Conversation
There was a problem hiding this comment.
🟢 GREEN! 🟢
This project looks great, Elzara and Charday!
I left some comments but overall they are meant as suggestions for your future coding rather than things that are "wrong" or anything like that! I think the big thing to think about is that sometimes your code lost a little bit of consistency on how you handled data from function to function! Other than that, it's simply a matter of cleaning some things up and you're good! Just things to think about as you keep writing python!
| # ******************************************************************************************* | ||
| # ****** Add assertions here to test that the correct movie was added to "watched" ********** | ||
| # ******************************************************************************************* | ||
| assert updated_data["watched"] == [{ |
There was a problem hiding this comment.
This looks good! I love the way you set up this assertion! A small change is that it can be a little clunky to write out an entire list. You could remove the list aspect by removing the square brackets and adding that to the left side of the assertion:
assert updated_data["watched"][0] ==
|
|
||
| @pytest.mark.skip() | ||
| assert updated_data["watched"] == [FANTASY_2, movie_to_watch] |
There was a problem hiding this comment.
Same kind of idea as before, since this is a small list, I would simply check to see if the item at a specific index of the list is what you are looking for, rather than checking to make sure the entire list looks the same! One way we could make this a little more generic is to check and see that the item at index -1 is what we're looking for since we know it will always be appended to the end of the list!
| # ************************************************************************************************** | ||
|
|
||
| @pytest.mark.skip() | ||
| assert INTRIGUE_3 in friends_unique_movies |
There was a problem hiding this comment.
This looks good too and I think it's a really good starting point for this particular test! One thing to think about however is what this test is actually testing! If we look at the name (test_friends_unique_movies_not_duplicated), it becomes clear that we will have the same movie in multiple friends watched lists! As a result, this test is looking specifically to see how many times that movie gets added. While checking to see if it's in the list you create works, are there other ways to make sure it isn't duplicated?
| @@ -52,13 +52,11 @@ def test_new_genre_rec_from_empty_friends(): | |||
| } | |||
| ] | |||
| } | |||
|
|
|||
| result = get_new_rec_by_genre(sonyas_data) | |||
| assert result == [] | |||
| if not title or not genre or not rating: | ||
| return None | ||
|
|
||
| return {"title": title, "genre": genre, "rating": rating} |
There was a problem hiding this comment.
The way y'all set up this function is really great. I love the guard clause/normal flow structure that you have set up here!
| for friend in user_data["friends"]: | ||
| for movie in friend["watched"]: | ||
| title = movie["title"] | ||
| if title not in user_movie_titles: | ||
| if title not in friends_movie_titles: | ||
| friends_movie_titles.add(title) | ||
| unique_friends_movies.append(movie) |
There was a problem hiding this comment.
The one small thing to think about here is how the not in statement works. It's going to have to loop through the collections to see if the specified titles exist within them. As a result, we end up with a potential for 4 nested loops here. I wonder if there is a way this could be refactored! Also, if we think about how the friends_movie_titles set works, we don't actually need to check and see if a title exists within it at all! If it's a set, we can just add it and any duplicates just won't make the cut!
| def get_available_recs(user_data): | ||
|
|
||
| # Get unique movies friends watched, but user hasn't | ||
| unique_movies = get_friends_unique_watched(user_data) |
There was a problem hiding this comment.
Great job using your function from a previous wave! Helper functions are always great!
| # Append a movie to a list of recommended movies | ||
| # if a movie is hosted by a subscription user has acces to | ||
| for movie in unique_movies: | ||
| host = movie["host"] |
There was a problem hiding this comment.
I'm just calling this out because it ends up being a little inconsistent from function to function. Sometimes you store data from the dictionary in a new variable, other times you don't. As you get more comfortable with programming, I would lean toward directly accessing the data, but more importantly, keep things consistent across your program!
|
|
||
| # Create list of recommended movies with most frequent genre | ||
| not_watched = get_friends_unique_watched(user_data) | ||
| recommended_movies = [ |
There was a problem hiding this comment.
Y'all killed it with these list comprehensions!
| if movie["title"] in favorite_movies | ||
| ] | ||
|
|
||
| return recommended_favorites |
There was a problem hiding this comment.
This function looks really great too! Good job using methods from before and as always, another good list comprehension!
No description provided.