Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zoisite - Jackie Aponte #40

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Zoisite - Jackie Aponte #40

wants to merge 9 commits into from

Conversation

japonte08
Copy link

No description provided.

Yiskah-S and others added 9 commits March 29, 2023 13:20
…movies in the user's watch list and a function to get the user's most watched genre.
… returns a list of the movies the user has watched that their friends haven't watched. Includes a function get_friends_unique_movies that returns a list of movies the user's friends have watched that they haven't watched. test_wave_03 updated. test_friends_unique_movies_not_duplicated also tests to make sure the correct movies are returned from get_friends_unique_watched.
…ecommended movies from friends watched movies if the user is subscribed to the movie's host and the user hasn't watched the movie yet.
…recommendations for the user if their friends have watched it and the movie genre is the most watched genre by user. the function get_rec_from_favorites returns a list of movies the user recommends if their friends haven't watched it and it is in their favorites list. The test test_new_genre_rec_from_empty_friends is completed.
@japonte08 japonte08 closed this Apr 3, 2023
@japonte08 japonte08 reopened this Apr 3, 2023
Copy link
Contributor

@yangashley yangashley left a comment

Choose a reason for hiding this comment

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

Nice job on Viewing Party, Jackie!

Your project shows that you're getting more fluent with working with nested data structures, iterating over them, and using conditional logic.

Nice job making frequent commits with descriptive commit messages!

Let me know if you have any questions.

Comment on lines +161 to +163
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
Copy link
Contributor

Choose a reason for hiding this comment

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

Nicely done

Comment on lines +189 to +191
assert movie_to_watch not in updated_data["watchlist"]
assert movie_to_watch in updated_data["watched"]
assert FANTASY_2 in updated_data["watched"]
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Comment on lines +59 to +61
assert len(recommendations) == 0
assert not recommendations
assert recommendations == []
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd only do one of these checks. assert recommendations == [] checks that the result is an empty list, while assert len(recommendations) == 0 checks only that recommendations is something reporting a length of 0.

If you really want to be sure that the return value is truly a list, then go with == [], but I'd tend towards the latter (we tend to be a little loose with types in Python).


if not (title and genre and rating):
return None
else:
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for this else statement. When possible we should try not to write extra statements or creating nesting when we don't need them to avoid bugs.

Also, we can directly return a literal dictionary without referencing it with a variable, like:
return { "title" : title, "genre" : genre, "rating" : rating}

Comment on lines +41 to +43
rating_list = []
if not user_data["watched"]:
return 0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider adding a guard clause before line 41 because we don't need rating_list` if we're going to return 0.0 when the watched list is empty.

Comment on lines +79 to +81
for friend in user_data["friends"]:
for movie in friend["watched"]:
if movie["title"] not in friends_watched_titles:
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider pulling this into a helper function, maybe like get_friends_movies()

if movie["title"] == title:
user_unique_movies.append(movie)

return user_unique_movies
Copy link
Contributor

Choose a reason for hiding this comment

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

Another way to approach this problem would be to use sets. You could create a set of a set of a friend's watched titles.

for movie in user_data["watched"]: 
    if movie["title"] not in friends_watched_set: 
        user_unique_movies.append(movie) return unique_list

Comment on lines +116 to +120
for title in friends_unique_titles:
for friend in user_data["friends"]:
for movie in friend["watched"]:
if movie["title"] == title and movie not in friends_unique_movies:
friends_unique_movies.append(movie)
Copy link
Contributor

Choose a reason for hiding this comment

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

When I see many for loops nested like this, I think about how we could pull values into variables so we don't get deeply indented levels of code. For example, instead of needing two for loops to get into friends' movies, could you create a list of all friends movies before this and then use that list here to save on a for loop?

Also, consider how you could use sets here too because you have for loops using the in operator on lists which can quickly lead to quadratic or even cubic time complexity.

You could create a set of user's watched movie titles and have the inverse logic of get_unique_watched.


# -----------------------------------------
# ------------- WAVE 4 --------------------
# -----------------------------------------

def get_available_recs(user_data):

friends_recommended_movies = get_friends_unique_watched(user_data)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice job reusing a method you already wrote.

Comment on lines +134 to +135
for movie in friends_recommended_movies:
if movie["host"] in user_data["subscriptions"]:
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

subscriptions = set(user_data["subscriptions"]) 
for movie in friends_recommended_movies: 
    if movie["host"] in subscriptions: 
        recommended_movies.append(movie)

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.

3 participants