Ocelots - Lisa Utsett #11
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,129 @@ | ||
| # ------------- WAVE 1 -------------------- | ||
|
|
||
| def create_movie(title, genre, rating): | ||
| if not title or not genre or not rating: | ||
|
There was a problem hiding this comment. Great use of the truthy/falsy values to check for empty strings or |
||
| return None | ||
| else: | ||
|
There was a problem hiding this comment. Since it doesn't change the result of our function, I recommend removing the If an |
||
| return { "title": title, | ||
| "genre": genre, | ||
| "rating": rating | ||
| } | ||
|
|
||
| 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 | ||
|
|
||
| def watch_movie(user_data, title): | ||
| for movie in user_data["watchlist"]: | ||
| if movie["title"] == title: | ||
| add_to_watched(user_data, movie) | ||
|
|
||
| user_data["watchlist"].remove(movie) | ||
| break | ||
|
There was a problem hiding this comment. Great use of |
||
| return user_data | ||
|
|
||
|
|
||
| # ----------------------------------------- | ||
| # ------------- WAVE 2 -------------------- | ||
| # ----------------------------------------- | ||
| def get_watched_avg_rating(user_data): | ||
| sum = 0.0 | ||
|
There was a problem hiding this comment. Something to note: the names |
||
| user_data_len = len(user_data["watched"]) | ||
|
Comment on lines
+33
to
+34
There was a problem hiding this comment. I would consider moving the instantiation of |
||
| if user_data_len >0: | ||
|
There was a problem hiding this comment. This is a little bit personal taste, but I often prefer to reverse the validation check and use it like a guard clause at the top of the function. This can let us take the main logic of our function out of an if-statement so it is prominent and clear rather than indented. For me, this makes code easier to read, but you may feel differently or work on a team that has guidelines for how they structure validation checks. We can also use the truthy/falsy value of if not user_data["watched"]:
return 0.0
sum = 0.0
for movie in user_data["watched"]:
sum += movie["rating"]
return sum / len(user_data["watched"]) |
||
| for movie in user_data["watched"]: | ||
| sum += movie["rating"] | ||
|
Comment on lines
+36
to
+37
There was a problem hiding this comment. Nice approach! Another way we could do this is using list comprehensions and the # The list comprehension inside the `sum` call pulls the value of the
# `rating` key out of each item in the list `user_data['watched']`
sum = sum(movie["rating"] for movie in user_data["watched"])
return sum / len(user_data["watched"]) |
||
|
|
||
| return sum/user_data_len | ||
|
There was a problem hiding this comment. The PEP8 guide on whitespace recommends we always place a single space on either side of an operator. This makes it easier for folks to visually process a line of code and see the groups of related statements in an expression. If you'd like more info, I suggest checking out the style guide for whitespace in expressions and statements: https://peps.python.org/pep-0008/#whitespace-in-expressions-and-statements |
||
| else: | ||
| return sum | ||
|
|
||
|
|
||
| def get_most_watched_genre(user_data): | ||
| most_watched_dict = {} | ||
| user_data_len = len(user_data["watched"]) | ||
|
|
||
| if user_data_len > 0: | ||
| for movie in user_data["watched"]: | ||
| if movie["genre"] in most_watched_dict: | ||
| most_watched_dict[movie["genre"]] += 1 | ||
| else: | ||
| most_watched_dict[movie["genre"]] = 0 | ||
|
Comment on lines
+49
to
+53
There was a problem hiding this comment. There's a tradeoff of using a little more space for another variable, but to make the code a little easier to read and reduce how many times we need to access for movie in user_data["watched"]:
movie_genre = movie["genre"]
if movie_genre in most_watched_dict:
most_watched_dict[movie_genre] += 1
else:
most_watched_dict[movie_genre] = 1
Comment on lines
+49
to
+53
There was a problem hiding this comment. There's an off by one error in how the genres are counted; if we want an accurate count, we would need to start the counter at 1 rather than 0. As an example, let's say we have a list with only one |
||
|
|
||
| most_watched_genre = max(most_watched_dict, key=most_watched_dict.get) | ||
|
There was a problem hiding this comment. Great use of a frequency map and |
||
| return most_watched_genre | ||
| else: | ||
| return None | ||
|
|
||
| # ----------------------------------------- | ||
| # ------------- WAVE 3 -------------------- | ||
| # ----------------------------------------- | ||
| def get_unique_watched(user_data): | ||
| unique_movies = [] | ||
| friends_movies = set() | ||
|
|
||
|
|
||
| for friend in user_data["friends"]: | ||
| for movie in friend["watched"]: | ||
| friends_movies.add(movie["title"]) | ||
|
|
||
| for user_movie in user_data["watched"]: | ||
| if user_movie["title"] not in friends_movies: | ||
| unique_movies.append(user_movie) | ||
|
|
||
| return unique_movies | ||
|
|
||
| def get_friends_unique_watched(user_data): | ||
| unique_movies = [] | ||
| user_movies = [] | ||
|
|
||
| for user_movie in user_data["watched"]: | ||
| user_movies.append(user_movie["title"]) | ||
|
Comment on lines
+79
to
+82
There was a problem hiding this comment. If we don't need complicated logic to transform data, a common way to fill a list in Python is using a list comprehension: user_movies = [movie["title"] for movie in user_data["watched"]]Here's a resource on list comprehensions if you want more info: https://realpython.com/list-comprehension-python/ |
||
|
|
||
| for friend in user_data["friends"]: | ||
| for movie in friend["watched"]: | ||
| if movie["title"] not in user_movies and movie not in unique_movies: | ||
| unique_movies.append(movie) | ||
|
|
||
| return unique_movies | ||
|
Comment on lines
+78
to
+89
There was a problem hiding this comment. There is nothing wrong with this organization, but I want to suggest a slight change that can help readers, especially in longer functions. Instead of declaring all our variables at the top of a function, I recommend creating them just before you need to use them. This help reduce needing to jump around the function or scroll up the page to remind ourselves of what a variable holds. user_movies = []
for user_movie in user_data["watched"]:
user_movies.append(user_movie["title"])
unique_movies = []
for friend in user_data["friends"]:
... |
||
|
|
||
| # ----------------------------------------- | ||
| # ------------- WAVE 4 -------------------- | ||
| # ----------------------------------------- | ||
|
|
||
| def get_available_recs(user_data): | ||
| friends_recs = [] | ||
| friends_unique_data = get_friends_unique_watched(user_data) | ||
|
|
||
|
|
||
| if len(friends_unique_data) >0 and len(user_data["watched"])> 0: | ||
| for movie in friends_unique_data: | ||
| if movie["host"] in user_data["subscriptions"]: | ||
| friends_recs.append(movie) | ||
|
Comment on lines
+100
to
+102
There was a problem hiding this comment. Nice algorithm! Another way we could approach filtering the # list comprehension
result = [movie for movie in friends_unique_data if movie["host"] in user_data["subscriptions"]]This line is a bit long, in practice we would split a statement like this across lines, use some extra variables, or shorten some naming to keep under the PEP8 guide of 79 characters max per line: # list comprehension
result = [movie for movie in friends_unique_data
if movie["host"] in user_data["subscriptions"]] |
||
|
|
||
| return friends_recs | ||
|
|
||
|
|
||
| # ----------------------------------------- | ||
| # ------------- WAVE 5 -------------------- | ||
| # ----------------------------------------- | ||
|
|
||
| def get_new_rec_by_genre(user_data): | ||
| rec_by_genre = [] | ||
| most_watched_genre = get_most_watched_genre(user_data) | ||
| recs = get_available_recs(user_data) | ||
| for movie in recs: | ||
| if movie["genre"] == most_watched_genre: | ||
| rec_by_genre.append(movie) | ||
|
|
||
| return rec_by_genre | ||
|
Comment on lines
+112
to
+119
There was a problem hiding this comment. Though this solution passes the current test suite, it isn't doing what the problem statement asks. The test
|
||
|
|
||
| def get_rec_from_favorites(user_data): | ||
| rec_from_favorites = [] | ||
| unique_watched = get_unique_watched(user_data) | ||
|
|
||
| if len(user_data["favorites"]) >0: | ||
| for movie in unique_watched: | ||
| if movie in user_data["favorites"]: | ||
| rec_from_favorites.append(movie) | ||
| return rec_from_favorites | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice assertion for the title! In this case where the test is checking individual keys of a dictionary, I recommend including assertions for all of the relevant keys.