Conversation
anselrognlie
left a comment
anselrognlie
left a comment
There was a problem hiding this comment.
Nice job!
All your tests are passing, and your code is well-organized. Great use of sets to get those constant time in lookups. There are a few more places you could apply the same strategy! Also, check out my comments about comprehension syntax. When used appropriately, it can make it really clear that we're extracting a piece of data from a list of records.
| # Assert | ||
| assert len(updated_data["watchlist"]) == 0 | ||
| assert len(updated_data["watched"]) == 1 | ||
| assert updated_data["watched"][0]["title"] == MOVIE_TITLE_1 |
| assert HORROR_1 in updated_data["watched"] | ||
| assert FANTASY_2 in updated_data["watched"] | ||
| assert FANTASY_1 in updated_data["watchlist"] |
| 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.
👍 There weren't any particular specifications as to the ordering of the movies in the list of results. So using an in lets us check for the movies we expect to see, without requiringus to know at what exact position each movie ended up. This lets us enforce the required behaviors in the test without being overly strict.
In fact, the checks from the previous test are exctly what we would need to add. Since this test already had a check for the list having 3 items, if we then check for 3 expected items and find them all, this also means that each item appears in the result only once (no duplication).
| recommendations = get_new_rec_by_genre(sonyas_data) | ||
|
|
||
| # Assert | ||
| assert len(recommendations) == 0 |
| def create_movie(title, genre, rating): | ||
| pass | ||
|
|
||
| if title and genre and rating: |
There was a problem hiding this comment.
It can help make code more readable if instead of checking for the conditions we need to run our main logic (here building the dictionary), we instead check for the conditions that would prevent the main logic (if any value is falsy).
Doing so can help us avoid indenting the main logic of our function, which gives it more emphasis that it's the main expected case. Here, this could look like
if not title or not genre or not rating:
return None
# unindented code to build and return movie dictionary| for movie in user_data["watched"]: | ||
| users_movies.add(movie["title"]) |
There was a problem hiding this comment.
We can also use comprehension syntax for something like this as well
users_movies = set(movie["title"] for movie in user_data["watched"])| for friend in user_data["friends"]: | ||
| for movie in friend["watched"]: | ||
| if movie["title"] in unique_movies: | ||
| if movie not in unique_watched: |
There was a problem hiding this comment.
Since unique_watched is a list, this in check must iterate through the contents to check whether the movie is there.
Here, we do need to build a list of movie detail records, not just the title. So unique_movies must be a list. However, if we still wanted to gain the constant in lookup even for the result list, we could create another set in which we store only the titles of the movies we've added to unique_movies, and look up a movie's title in that set before adding it to the result list, and adding the title to the set if we do end up adding a movie to the result list.
As it is, doing the in checks (which are linear) in a loop (essentially linear) gives us an overall quadratic time complexity. But we could get this down to linear by building that extra set, since the operations in the loop would become constant.
| def get_available_recs(user_data): | ||
| recommended_movies = [] | ||
| host_list = user_data["subscriptions"] | ||
| unique_watched = get_friends_unique_watched(user_data) |
There was a problem hiding this comment.
👍 Nice use of your wave 3 method to simplify the logic here.
| # ----------------------------------------- | ||
| # ------------- WAVE 5 -------------------- | ||
| # ----------------------------------------- | ||
| def get_new_rec_by_genre(user_data): |
There was a problem hiding this comment.
👍 Another nice example of reusing your previous methods.
Notice that the construction of recs_by_genre could leverage comprehension syntax!
| recs_from_favs = [] | ||
|
|
||
| for movie in unique_recs: | ||
| if movie in user_data["favorites"]: |
There was a problem hiding this comment.
Like above, we could build a set of the titles in the user's favorites data to gain the benefit of those constant time in lookups!
2 participants
No description provided.