C18 Tigers Marie Keefer viewing-party#157
Conversation
spitsfire
left a comment
spitsfire
left a comment
There was a problem hiding this comment.
Good job, Marie! Your code was easy to read and understand.
I've added some feedback throughout your project to point out some places we can refactor in order to follow common conventions and standard practices when it comes to Python.
Things to consider as you work on future projects:
- Consider putting your conditionals that catch edge cases up at the top. These are referred to as guard clauses.
- If you have time over break, I recommend finishing up wave 5 and pushing it to GitHub!
| movie_dict = {} | ||
| if title and genre and rating: | ||
| movie_dict.update({"title": title}) | ||
| movie_dict.update({"genre": genre}) | ||
| movie_dict.update({"rating": rating}) | ||
| return movie_dict | ||
| else: | ||
| return None |
There was a problem hiding this comment.
I'd suggest flipping the sense of the condition around to handle the special case (that something is falsy) quickly and get out, rather than having that dangling else at the end. This arrangement is referred to as a guard clause or a guard condition.
Notice that it lets us move the main focus of the function (creating the new dict) out of an indented block, letting it stand out more clearly.
def create_movie(movie_title, genre, rating):
if not movie_title or not genre or not rating:
return None
movie_dict = {'title':movie_title, 'genre':genre, 'rating':rating}
return movie_dict| else: | ||
| return None | ||
|
|
||
| def add_to_watched(user_data, movie): |
| return user_data | ||
|
|
||
|
|
||
| def add_to_watchlist(user_data, movie): |
| return user_data | ||
| return user_data |
There was a problem hiding this comment.
we only need one return statement. But which one? Well, it depends! Do we want to immediately end the function once we've appended and removed one movie? Is there a possibility there could be duplicates of the same movie inside "watched" and "watchlist"? Maybe!
Either one of these works in this context, but do pick only one
| if len(rating_list) == 0: | ||
| return 0.0 | ||
| else: | ||
| final_rating = rating_total / len(rating_list) | ||
| return final_rating |
There was a problem hiding this comment.
Nice job handling the empty list case. Consider putting this check at the top as a guard clause in order to get it out of the way, which lets the main logic not be under a conditional.
if len(movies) < 1: # there are many ways we can check to see if movies is falsy
return 0.0
# remaining logic goes here| genre_dict[movie_dict["genre"]] += 1 | ||
| else: | ||
| genre_dict[movie_dict["genre"]] = 1 | ||
| best_genre = max(genre_dict, key=genre_dict.get) |
There was a problem hiding this comment.
good job! We talked about how this worked in the project review, but do you feel like you can explain this in your own words? When pulling in code from beyond what we've talked about, it's great if you could include a comment about the research that you did, and how you think it works.
Something to think about in future projects!
| for i in range(len(user_data["watched"])): | ||
| movie = user_data["watched"][i] | ||
| user_watched_list.append(movie) |
There was a problem hiding this comment.
this would make for a good helper function so we don't have to repeat code, like on lines 75-77
| for i in range(len(user_data["friends"])): | ||
| for movie in user_data["friends"][i]["watched"]: | ||
| friends_movie_list.append(movie) | ||
|
|
||
| for i in range(len(user_data["watched"])): | ||
| movie = user_data["watched"][i] | ||
| user_watched_list.append(movie) |
There was a problem hiding this comment.
aha! we've done this once already! Sounds like a job for a helper function!
| # ------------- WAVE 4 -------------------- | ||
| # ----------------------------------------- | ||
| def get_available_recs(user_data): | ||
| friends_unique_movies = get_friends_unique_watched(user_data) |
| # ----------------------------------------- | ||
| # ------------- WAVE 4 -------------------- | ||
| # ----------------------------------------- | ||
| def get_available_recs(user_data): |
2 participants
waves 1, 2, 3 and 4 are complete I didn't finish 5