Sharks - Jeannie Zheng #108
Conversation
There was a problem hiding this comment.
It's clear you worked hard on this, Jeannie! I've left some suggestions in your code below. Some things to consider as you gear up for future projects:
Get to know the test data! Really understand what kind of data is being passed to the function you are building.
- This will help you make more descriptive variable names that tell other coders what's being operated on or compared or appended, etc.
- It will also help you confidently iterate through the data you need without extra for loops or variables you may not need
Also don't be afraid to code review with others! Other Adies can inspire new approaches to solving a problem
| new_movie = {"title": title, | ||
| "genre": genre, | ||
| "rating": rating} | ||
| if title == None or genre == None or rating == None: | ||
| return None | ||
| else: | ||
| return new_movie |
There was a problem hiding this comment.
I'd suggest flipping the order of the conditions around to handle the special case (ie, checking if something is falsy) first, so that the code can quickly check and get out of there. 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 a dictionary from the parameters) out of an indented block, letting it stand out more clearly.
Another thing is, if we flip the conditionals, the program doesn't waste memory space making a dictionary that won't be returned if the parameters turn out to be falsy
| new_movie = {"title": title, | |
| "genre": genre, | |
| "rating": rating} | |
| if title == None or genre == None or rating == None: | |
| return None | |
| else: | |
| return new_movie | |
| if title == None or genre == None or rating == None: | |
| return None | |
| new_movie = { | |
| "title": title, | |
| "genre": genre, | |
| "rating": rating | |
| } | |
| return new_movie | |
| watched_move_list = {"watched": [movie]} | ||
| return watched_move_list |
There was a problem hiding this comment.
Careful here! We don't want to accidentally overwrite the user_data's "watched" list that's already been supplied to us in the tests in test_wave_01.py. It did pass the tests, but unfortunately the tests did not' account for "watched" lists with more than one movie in it.
Make sure we use the user_data instead, update it there, then return the user_data
| watched_move_list = {"watched": [movie]} | |
| return watched_move_list | |
| user_data["watched"].append(movie) | |
| return user_data |
| watchlist_list = {"watchlist": [movie]} | ||
| return watchlist_list |
There was a problem hiding this comment.
Same as above! Let's make sure we update user_data instead of creating a new dictionary.
| for i in range(len(user_data["watchlist"])): | ||
| if user_data["watchlist"][i]["title"] == movie: | ||
| remove_movie = user_updated_data["watchlist"].pop(i) | ||
| user_updated_data["watched"] += [remove_movie] |
There was a problem hiding this comment.
nice! this works just fine! One thing you could think about is using for movie in user_data["watchlist"], which would make some of the variables a little short and easier to read. We could also use a break at the end so that we aren't running the for loop more time than necessary
Another thing, it's more common to use append here when adding only one item. This still works, though. Here's some examples of the differences between += and append
https://www.codecademy.com/forum_questions/559a2e9576b8fec400000392
| for i in range(len(user_data["watchlist"])): | |
| if user_data["watchlist"][i]["title"] == movie: | |
| remove_movie = user_updated_data["watchlist"].pop(i) | |
| user_updated_data["watched"] += [remove_movie] | |
| for movie in user_data["watchlist"]: | |
| if movie["title"] == movie_title: | |
| user_updated_data["watched"].append(movie) | |
| user_updated_data["watchlist"].remove(movie) |
| for key, value in user_data.items(): | ||
| for information in value: | ||
| counter += information["rating"] |
There was a problem hiding this comment.
Since we know the key we want to look into already, let's skip the first loop completely. There might be only one key in this particular dictionary, but we can't always assume. So let's go right to user_data["watched"]
| for key, value in user_data.items(): | |
| for information in value: | |
| counter += information["rating"] | |
| for movie in user_data["watched"]: | |
| counter += movie["rating"] | |
| friends_list = get_friends_unique_watched(user_data) | ||
| users_list = get_unique_watched(user_data) |
There was a problem hiding this comment.
yes! 👍 awesome job reusing functions you've already made again!!
| for item in friends_list: | ||
| for details in item: | ||
| if item["host"] in subscriptions: | ||
| recommend_list.append(item) | ||
|
|
||
| newlist = [] | ||
| for i in recommend_list: | ||
| if i not in newlist: | ||
| newlist.append(i) |
There was a problem hiding this comment.
As before, we could combine these into one and save ourselves some memory space
| friends_list = get_friends_unique_watched(user_data) | ||
| most_watched_genre = get_most_watched_genre(user_data) |
|
|
||
| def get_new_rec_by_genre(user_data): | ||
| new_rec_genre = [] | ||
| friends_list = get_friends_unique_watched(user_data) |
There was a problem hiding this comment.
I think to save you some filtering we could use get_avilable_recs method instead!
| friends_list = get_friends_unique_watched(user_data) | |
| recs_list = get_available_recs(user_data) |
| recommend_list = [] | ||
| user_favorite = [] | ||
| user_unique_list = get_unique_watched(user_data) | ||
|
|
||
| for movie in user_data["favorites"]: | ||
| user_favorite. append(movie) | ||
|
|
||
| for movie in user_unique_list: | ||
| if movie in user_favorite: | ||
| recommend_list.append(movie) | ||
| return recommend_list |
There was a problem hiding this comment.
We could combine some of these by using get_available_recs and then loop through them, then check against user's favorites!
| recommend_list = [] | |
| user_favorite = [] | |
| user_unique_list = get_unique_watched(user_data) | |
| for movie in user_data["favorites"]: | |
| user_favorite. append(movie) | |
| for movie in user_unique_list: | |
| if movie in user_favorite: | |
| recommend_list.append(movie) | |
| return recommend_list | |
| recommend_list = get_available_recs(user_data) | |
| fav_rec_list = [] | |
| for movie in recommend_list: | |
| if movie in user_data["favorites"]: | |
| fav_rec_list.append(movie) | |
| return fav_rec_list |
No description provided.