snow leopards arika agnew #151
Conversation
There was a problem hiding this comment.
Great work on your first Unit 1 project, Arika! 🟢 for Viewing Party!
I called out a couple of places that you could refactor your code to make it a bit more pythonic. I also commented in a few places about making your variable names a little more descriptive which helps make code more readable.
Also, good work making many small commits and writing descriptive messages.
| 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 |
There was a problem hiding this comment.
Nice work writing additional assertions to make your test more robust.
| assert updated_data["watched"][1]["title"] == MOVIE_TITLE_1 | ||
| assert updated_data["watched"][1]["genre"] == GENRE_1 | ||
| assert updated_data["watched"][1]["rating"] == RATING_1 |
| movie_info = [title,genre,rating] | ||
| if None in movie_info: |
There was a problem hiding this comment.
A more Pythonic way to do this check would be like:
if not title and not genre and not rating:
return NoneIt also conveys your intent a little more strongly, if other devs are reading your code they could see that if any of these values are invalid, we return None.
And since you have this check you don't need an else block because if any of the arguments were invalid, then the function would have returned and you can delete the else block.
Also, you can return a dictionary literal instead of storing the values in a variable because we don't have another place in this method where we need to use the variable.
return {
"title": title,
"genre": genre,
"rating": rating
}| return movie_dict | ||
|
|
||
| def add_to_watched(user_data,movie): | ||
| user_data = {} |
There was a problem hiding this comment.
user_data is already a dictionary with values in it. If we reassign it to an empty dict, then we lose the data that was part of the argument.
We don't need to make user_data a dictionary, since it already is one. It can help to use the debugger to inspect arguments so we know what we're working with when we write our methods.
|
|
||
| def add_to_watched(user_data,movie): | ||
| user_data = {} | ||
| user_data["watched"] = [movie] |
There was a problem hiding this comment.
user_data["watched"] returns a list of dictionaries representing watched movies. If we reassign the value like this, we overwrite the existing list. So instead of adding movie to the end of watched list, your code just makes the watched list one movie.
You could refactor this to user_data["watched"].append(movie)
| for friend in user_data["friends"]: | ||
| if movie in friend["watched"]: | ||
| same_movies.append(movie) | ||
| print(movie, same_movies) |
There was a problem hiding this comment.
Remember to remove debugging print statements when you're done with them.
|
|
||
| # ----------------------------------------- | ||
| # ------------- WAVE 3 -------------------- | ||
| # ----------------------------------------- | ||
|
|
||
| def get_unique_watched(user_data): | ||
| my_movies = [] | ||
| same_movies = [] |
There was a problem hiding this comment.
Consider renaming same_movies to something more descriptive like "friends_watched_movies"
| for movies in user_data["watched"]: | ||
| if movies not in same_movies: | ||
| my_movies.append(movies) | ||
| return my_movies |
| for movie in user_data["watched"]: | ||
| for friend in user_data["friends"]: |
There was a problem hiding this comment.
user_data["watched"] and user_data["friends"] are two different lists. We don't need line 72 in order to loop through friends and their watched movies. You can delete line 72 and your loop still runs as it should
| # ----------------------------------------- | ||
| # ------------- WAVE 4 -------------------- | ||
| # ----------------------------------------- | ||
| def get_available_recs(user_data): | ||
| recommended_movies =[] | ||
| friends_watched = get_friends_unique_watched(user_data) |
There was a problem hiding this comment.
Nice job reusing a method you already wrote :)
No description provided.