Skip to content

Diana Santini Pine#87

Open
DLozanoC wants to merge 2 commits intoAda-C16:masterfrom
DLozanoC:master
Open

Diana Santini Pine#87
DLozanoC wants to merge 2 commits intoAda-C16:masterfrom
DLozanoC:master

Conversation

@DLozanoC
Copy link

No description provided.

Copy link

@jbieniosek jbieniosek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work on this project Diana. There are some significant logical bugs as well as some places where you could simplify your code by adding helper functions. This project is yellow.

Comment on lines +31 to +40
for movie in user_data["watchlist"]:
if movie["title"] == title:
watched_dict = movie
break
else:
return user_data
user_data["watched"].append(watched_dict)
user_data["watchlist"].remove(watched_dict)

return user_data

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works, but the flow is a little awkward. I would recommend restructuring for readability:

Suggested change
for movie in user_data["watchlist"]:
if movie["title"] == title:
watched_dict = movie
break
else:
return user_data
user_data["watched"].append(watched_dict)
user_data["watchlist"].remove(watched_dict)
return user_data
for movie in user_data["watchlist"]:
if movie["title"] == title:
watched_dict = movie
user_data["watched"].append(watched_dict)
user_data["watchlist"].remove(watched_dict)
return user_data

for movie in user_data["watched"]:
user_rating.append(movie["rating"])

average = sum(user_rating)/len(user_rating)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice solution! As above, this works, but the layout implies different logic than what is actually here. I would recommend eliminating the else and moving lines 49-51 to the same tab level as this line.

for movie in user_data["watched"]:
most_watched.append(movie["genre"])

return max(most_watched, key = most_watched.count)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would you solve this without using the max function?

#Wave 3 Tests 1 2
def get_unique_watched(user_data):

#Creating variables that are not exactly necessary but make my code easier to read (at least to me)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Helper variables are a great way to improve readability!

Comment on lines +96 to +103
friends_movies_first = friends_movies[0]
friends_movies_second = friends_movies[1]
# adding each item(dictionary) to friends_movies_list
for dictionary in friends_movies_first["watched"]:
friends_movies_list.append(dictionary)

for dictionary in friends_movies_second["watched"]:
friends_movies_list.append(dictionary)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a logical bug in this section. This only handles the first two friends in the friends list - what happens if there are more? Note - your code in get_unique_watched on lines 77-79 does similar work without the bug.


for movie in user_movies:
for item in unique_friends_list:
if movie["title"] in item.values():

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a logical bug on this line. What if there is a movie with the title "Intrigue", and a movie in the friends list has a title and a genre, and the genre is "Intrigue"? This conditional will be true, and that movie will be removed from unique_friends_list even though the titles don't match. How could you compare just the titles?

# Wave 4 Tests 1 2 3 4
def get_available_recs(user_data):

recommendations = get_friends_unique_watched(user_data)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great code reuse!


recommendations = get_friends_unique_watched(user_data)

for movie in range(len(recommendations)):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This outer loop is not doing anything.

#removes duplicates
rec_movies = [i for n, i in enumerate(rec_movies) if i not in rec_movies[n + 1:]]

return rec_movies

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a logical bug in this function. This function does not check for the most popular genre or do any checking against the genre. It will produce the same output as 'get_friends_unique_watched'.

Comment on lines +179 to +183
for movie in user_favorites:
if movie not in friends_list:
favorites.append(movie)

return favorites No newline at end of file

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants