Conversation
…nctions, including getting most watched genre funct to pass all wave 2 tests
…nique_watched', all wave three tests passed
spitsfire
left a comment
spitsfire
left a comment
There was a problem hiding this comment.
Great job, Kindra! You had creative ideas that helped you solve each wave, so congrats passing all the tests.
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:
for i in rangeloops are good when you need to access or use the index position value in your program. If you are going through a list of objects or a collection of values that have a common theme (list of movies, list of dog breeds, etc.), consider directly iterating over the list likefor genre in genre_list- for loops will increment on their own. We don't need to manually increment
iwithi += 1 - if we are returning one value in a function, we don't need parentheses
| if None in information: | ||
| return None | ||
| else: |
There was a problem hiding this comment.
Think about putting this at the top of the function first. This way it handles the edge cases or special cases quickly and gets 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.
It also saves us from creating space in memory, defining variables, etc. that we are simply going to throw away because the parameters didn't meet our rules.
information = [title,genre,rating]
if None in information:
return None
# the rest of your code here| information = [title,genre,rating] | ||
| if None in information: |
There was a problem hiding this comment.
very creative way to check if all variables are valid. if... in has a for loop running under the hood to check each item in the list, so even though it looks cleaner than several compound conditions, it may not be as efficient.
Hard to say, though! The data we use is so small, it certainly doesn't matter on this scale!
| movie_dict = {} | ||
| movie_dict["title"]= title | ||
| movie_dict["genre"] = genre | ||
| movie_dict["rating"] = rating |
There was a problem hiding this comment.
this works just fine! We could also do this all at once:
| movie_dict = {} | |
| movie_dict["title"]= title | |
| movie_dict["genre"] = genre | |
| movie_dict["rating"] = rating | |
| movie_dict = { | |
| movie_dict["title"]: title, | |
| movie_dict["genre"]: genre, | |
| movie_dict["rating"]: rating | |
| } |
Now, obviously this isn't one line like this, so we aren't saving space. But we can do it all in one expression instead of 4.
| return movie_dict | ||
|
|
||
| def add_to_watched(user_data, movie): | ||
| user_data["watched"] = [movie] |
There was a problem hiding this comment.
careful here! You are re-assigning user_data["watched"] value, not appending to the list already inside.
What if this same user adds another movie to "watched"? It would overwrite the first movie with the new movie, instead of appending it to the list already declared in user_data["watched"]
| user_data["watched"] = [movie] | |
| user_data["watched"].append(movie) |
| user_data["watchlist"] = [movie] | ||
| return user_data |
There was a problem hiding this comment.
same as above! We want to append to "watchlist" list, not overwrite it with a new one
|
|
||
| def get_available_recs(user_data): | ||
| movie_recs = [] | ||
| list_of_movie_dictionaries_user_has_not_watched_but_friend_has = get_friends_unique_watched(user_data) |
| movie_recs = [] | ||
| list_of_movie_dictionaries_user_has_not_watched_but_friend_has = get_friends_unique_watched(user_data) | ||
| for subscription in user_data["subscriptions"]: | ||
| for dictionary in list_of_movie_dictionaries_user_has_not_watched_but_friend_has: |
There was a problem hiding this comment.
what is dictionary represent in this instance? could we call it something more descriptive, like movie?
| if users_most_frequently_watched_genre == None: | ||
| movie_recs_by_genre = [] |
There was a problem hiding this comment.
I think we can get rid of this altogether since it is already set up as an empty list on line 160
| if users_most_frequently_watched_genre == dictionary["genre"]: | ||
| movie_recs_by_genre.append(dictionary) | ||
|
|
||
| return(movie_recs_by_genre) |
There was a problem hiding this comment.
| return(movie_recs_by_genre) | |
| return movie_recs_by_genre | |
| for dictionary in list_of_movie_dictionaries_user_has_watched_but_friends_have_not: | ||
| if favorite["title"] == dictionary["title"]: | ||
| movie_recs_from_favorites.append(dictionary) | ||
| return(movie_recs_from_favorites) |
There was a problem hiding this comment.
| return(movie_recs_from_favorites) | |
| return movie_recs_from_favorites | |
2 participants
No description provided.