Nancy - Lions by nancy-lee89 · Pull Request #161 · Ada-C18/viewing-party

nancy-lee89 · 2022-09-23T16:59:33Z

Even though I completed all the waves, after reviewing the code I completely forgot how my group and I ended up solving Wave 2.

alope107

Hi Nancy, great job! There's a lot of really good logic here and your comments especially in the later waves are super helpful. I really appreciate you being so forthcoming about the parts of the code you still have challenges understanding. The code here makes for a green project, but we can chat about how to go back and make sure you can get a fuller understanding of the earlier waves 🙂

alope107 · 2022-09-30T18:14:49Z

    # Assert
    assert len(updated_data["watchlist"]) == 1
    assert len(updated_data["watched"]) == 2
+    assert updated_data["watched"][1]["title"] == MOVIE_TITLE_1


Consider checking that the whole movie matches instead of just the title so that we make sure the other attributes are copied over as well.

alope107 · 2022-09-30T18:15:42Z

+    actual = get_new_rec_by_genre(sonyas_data)
+    assert actual == []


Great act and assert!

alope107 · 2022-09-30T18:18:35Z

+    if title and genre and rating:
+        new_movie = {}
+        new_movie["title"] = title
+        new_movie["genre"] = genre
+        new_movie["rating"] = rating
+        return new_movie
+    if not title or not genre or not rating:
+        return None


Nice logic! A few quick notes:

Consider making the whole dictionary at once:

new_movie = { "title": title, "genre": genre, "rating": rating }

Consider removing line 10. If line 4 is False, then line 10 must be True, so we don't need to check it again.

alope107 · 2022-09-30T18:19:02Z

+def add_to_watched(user_data, movie):
+    user_data["watched"].append(movie)
+    return user_data
+
+def add_to_watchlist(user_data, movie):
+    user_data["watchlist"].append(movie)
+    return user_data


Nice and concise functions!

alope107 · 2022-09-30T18:21:15Z

+def get_watched_avg_rating(user_data):
+    sum = 0
+    average = 0
+
+    for i in range(len(user_data["watched"])):
+        sum += (user_data["watched"][i]["rating"])
+        average = sum/len(user_data["watched"])
+    return average


Consider using a for-each instead of a for here; the index is unimportant.

The parentheses on line 39 are unneeded.

Consider calculating the average only once at the end instead of at every iteration of the loop.

alope107 · 2022-09-30T19:12:47Z

+    for item in friends_movies: # we are accesing the list we created of movies watched by friends
+        if item not in user_movies: # it the friends movie is not in the movies the user has seen
+            if item not in unique: # if that movie is also not in our list of unique movies, we add to unique
+                if item["host"] in user_data["subscriptions"]: # if streaming service is in subscription
+                    unique.append(item) # if streaming service is found, add to unique


Great logic! Consider combining the if statements here.

alope107 · 2022-09-30T19:18:53Z

+    if len(user_movies) == 0:
+        return friends_movies # if the list of user is completely empty
+
+    max(set(most_genre), key=most_genre.count) # will get the most watched genre, will 


This is neat and works! That being said, I'd encourage you as we're learning to also be thinking about how to do things in the less fancy way.

alope107 · 2022-09-30T19:19:29Z

+def get_new_rec_by_genre(user_data):
+    user_movies = []
+    friends_movies = []
+    most_genre = []
+    recommended_movies = []
+
+    for movie in user_data["watched"]: # this is to help create the user watch list
+        user_movies.append(movie) #user_movies has a list of the movies the user has seen
+        most_genre.append(movie["genre"]) # to get to the genre
+
+    if len(user_movies) == 0:
+        return friends_movies # if the list of user is completely empty
+
+    max(set(most_genre), key=most_genre.count) # will get the most watched genre, will 


Consider re-using earlier functions as helpers here.

alope107 · 2022-09-30T19:25:24Z

+    for item in friends_movies: # we are accesing the list we created of movies watched by friends
+        if item not in user_movies: # it the friends movie is not in the movies the user has seen
+            if item not in most_genre: # if that movie is also not in our list of unique movies, we add to unique
+                if item["host"] in user_data["subscriptions"]: # if streaming service is in subscription
+                    if most_genre[0] in item["genre"]: # if the most popular genre is in the friends dict list
+                        recommended_movies.append(item) # if streaming service is found, add to recommended movies
+    return recommended_movies


I like a lot of the logic here! I think there's a couple of lines that might not be doing what we hope though.

Line 182 checks whether a movie dictionary is not inside a string. This will always be True (it's impossible for a string to hold a dictionary inside it, strings only hold letters, numbers, etc.)

Line 184 checks whether the first letter of the most common genre is in the movie's genre. This might return a false positive. For example, if most_genre was "fantasy" and the item's genre was "sci-fi", this if statement would incorrectly consider it a match because the first letter of most_genre, 'f', is present in "sci-fi".

alope107 · 2022-09-30T19:27:23Z

+    for item in user_data["favorites"]: # this accesses favorites in the user data
+        if item not in friend_movies: # if that favorite is not in friends movies
+            recommended_movies.append(item) # append the item to the recommended movies


Great logic!

alope107

Thanks for revisiting this! This logic looks great!

alope107 · 2022-10-18T16:21:38Z

+def get_watched_avg_rating(user_data):
+    count = 0
+    average_rating = 0
+
+    for i in range(len(user_data["watched"])):
+        count += (user_data["watched"][i]["rating"])
+        average_rating = count/len(user_data["watched"])
+    return average_rating


This works great! You could also consider using a a for-each loop here:
for movie in user_data["watched"]

Then, we could just get movie["rating"] instead of user_data["watched"][i]["rating"], a lot easier to work with!

alope107 · 2022-10-18T16:24:43Z

+def get_most_watched_genre(user_data):
+    genre_fave = [] # ['Fantasy', 'Fantasy', 'Fantasy', 'Action', 'Intrigue', 'Intrigue']
+    count = 0
+    current_highest = ""
+
+    for users_list in user_data["watched"]:
+        genre_fave.append(users_list["genre"])
+
+    for element in genre_fave:
+        if count < genre_fave.count(element): # count = 0 vs action=1
+            current_highest = element
+            count = genre_fave.count(element) # =1
+
+    if len(user_data["watched"]) == 0:
+        return None
+
+    return current_highest


This works great! A few notes:

Consider putting lines 57-58 at the beginning of the function. It's often a good idea to do our input validation at the beginning.

I like the use of the for-each loop on line 49, but I think the variable name is a bit misleading. user_list is actually a dictionary representing a movie, not a user or a list.

nancy-lee89 added 6 commits September 17, 2022 12:04

wave one completed

5c99229

passed first part of wave 2

f16a4a9

updated unique watched to use a list instead of set

43dc648

finished wave 3

fdbdc7d

passed all of the tests for wave 4

71404ad

finished wave 5

53c05ed

alope107 reviewed Sep 30, 2022

View reviewed changes

nancy-lee89 added 3 commits October 11, 2022 10:52

working from scratch on wave 2 on my own

118b456

finished part 1 and half of part 2 from wave 2

bd2146b

redid wave 2 with the help of tutor

0623e86

alope107 reviewed Oct 18, 2022

View reviewed changes

		actual = get_new_rec_by_genre(sonyas_data)
		assert actual == []

Conversation

nancy-lee89 commented Sep 23, 2022

Uh oh!

alope107 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alope107 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants